On Tue, Oct 10, 2023 at 10:49:04AM +0000, Richard Biener wrote:
> The following fixes a mistake in count_nonzero_bytes which happily
> skips over stores clobbering the memory we load a value we store
> from and then performs analysis on the memory state before the
> intermediate store.
> 
> The patch implements the most simple fix - guarantee that there are
> no intervening stores by tracking the original active virtual operand
> and comparing that to the one of a load we attempt to analyze.
> 
> This simple approach breaks two subtests of gcc.dg/Wstringop-overflow-69.c
> which I chose to XFAIL.

This function is a total mess, but I think punting right after the
gimple_assign_single_p test is too big hammer.
There are various cases and some of them are fine even when vuse is
different, others aren't.
The function at that point tries to handle CONSTRUCTOR, MEM_REF, or decls.

I don't see why the CONSTRUCTOR case couldn't be fine regardless of the
vuse.  Though, am not really sure when a CONSTRUCTOR would appear, the
lhs would need to be an SSA_NAME, so wouldn't for vectors that be a
VECTOR_CST instead, etc.?  Ah, perhaps a vector with some non-constant
element in it.
The MEM_REF case supposedly only if we can guarantee nothing could overwrite
it (so MEM_REF of TREE_STATIC TREE_READONLY could be fine, STRING_CST too,
anything else is wrong - count_nonzero_bytes_addr uses the
get_stridx/get_strinfo APIs, which for something that can be changed
always reflects only the state at the current statement.  So, e.g. the
get_stridx (exp, stmt) > 0 case in count_nonzero_bytes_addr is when
the caller must definitely punt if vuse is different.
Then for decls, again, CONST_DECLs, DECL_IN_CONSTANT_POOLs are certainly
fine.  Other decls for which ctor_for_folding returns non-error_mark_node
should be fine as well, I think ctor_useable_for_folding_p is supposed
to verify that.  STRING_CSTs should be fine as well.

So maybe pass the vuse down to count_nonzero_bytes_addr and return false
in the idx > 0 case in there if gimple_vuse (stmt) != vuse?

        Jakub

Reply via email to