https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92152
--- Comment #8 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #4)
> OK, so this happens in visit_reference_op_store which hits
>
> if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "Store matched earlier value, "
> "value numbering store vdefs to matching vuses.\n");
>
> changed |= set_ssa_val_to (vdef, SSA_VAL (vuse));
>
> where I actually wondered if I could trigger wrong-code with it. The
> counter-measure is
>
> /* If the TBAA state isn't compatible for downstream reads
> we cannot value-number the VDEFs the same. */
> alias_set_type set = get_alias_set (lhs);
> if (vnresult->set != set
> && ! alias_set_subset_of (set, vnresult->set))
> resultsame = false;
>
> but here we have v.i and v.f both of which get the alias-set of the union.
>
> We later disambiguate u.i against *f via aliasing_component_refs_p which
> correctly assesses that if u.i is the active union member then the load
> via *f cannot alias it.
>
> On x86_64 we optimize away the v.f store as redundant but then still
> don't CSE because somehow aliasing_component_refs_p doesn't get it in
> the case both fields are not of the same size.
>
> So I'm not entirely sure it isn't aliasing_component_refs_p that is to blame.
> At least it should make the testcase also fail on x86_64...
>
> Honza?
In particular for
/* If we have two type access paths B1.path1 and B2.path2 they may
only alias if either B1 is in B2.path2 or B2 is in B1.path1.
But we can still have a path that goes B1.path1...B2.path2 with
a part that we do not see. So we can only disambiguate now
if there is no B2 in the tail of path1 and no B1 on the
tail of path2. */
I think we have to consider B.path as having components if it accesses
a field of a union, also recursively in case you have
union { struct S s; struct U u; };. Honza, the type_has_components_p
check in
if (compare_type_sizes (TREE_TYPE (ref2), type1) >= 0
&& (!end_struct_ref1
|| compare_type_sizes (TREE_TYPE (ref2),
TREE_TYPE (end_struct_ref1)) >= 0)
&& type_has_components_p (TREE_TYPE (ref2))
&& (base1_alias_set == ref2_alias_set
|| alias_set_subset_of (base1_alias_set, ref2_alias_set)))
is a compile-time optimization? Because the alias subset check of course
would trigger here since ref2_alias_set is not the alias-set of
TREE_TYPE (ref2) but of one of its bases. OTOH we have a similar situation
for TYPE_NONALIASED_COMPONENTS where this is actually an optimization
which would be pessimized if we take it without the type_has_components_p
check.
That is, unless of course you are exploiting the active union member
thing and the GCC extension allowing type-punning only if the union
type is visible in the access. That conflicts with the desire to do
redundant store removal here since the store changes the active union
member but the guard that the alias effect is the same only considers
TBAA via alias-sets, which for unions use the alias-set of the union,
not via access path disambiguation ... :/
So you could indeed say this is a case of redundant store removal that's
bogus. IIRC ICF faces a similar issue of checking whether two refs behave
the same with respect to alias queries against all possible other refs...
Note that when I decided to make union accesses have the alias-set of the
union that implicitely allows type-punning when one of the refs have
the union visible as opposed to our documentation which could be read in
a way that both need to have it. So I'm somewhat leaning towards adjusting
path-based disambiguation here because that likely has the least impact
on overall code generation. The code already handles view-converts and
bit-field-refs this way so it's a quite natural extension.
Testing such patch.
