[Bug target/111591] ppc64be: miscompilation with -mstrict-align / -O3

["linkw at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111591

--- Comment #26 from Kewen Lin <linkw at gcc dot gnu.org> ---
(In reply to Richard Biener from comment #25)
> (In reply to Kewen Lin from comment #24)
> > (In reply to Richard Biener from comment #22)
> > > I see the mems properly get their base adjusted:
> > > 
> > > (insn 384 383 0 (set (mem/c:V2DI (plus:DI (reg/f:DI 112 virtual-stack-vars)
> > >                 (const_int 16 [0x10])) [7 MEM[(struct Vec128D.30433 *)_10]+0
> > > S16 A128])
> > >         (reg:V2DI 616)) -1
> > >      (nil))
> > > 
> > > vs.
> > > 
> > > (insn 389 388 390 (set (reg:HI 619)
> > >         (mem/c:HI (plus:DI (reg/f:DI 112 virtual-stack-vars)
> > >                 (const_int 16 [0x10])) [4 MEM[(struct Vec128D.30212
> > > *)_10].rawD.30221[0]+0 S2 A128])) "test.cc":218:14 -1
> > >      (nil))
> > > 
> > > both are based off a fake _10.  But we get alias sets 7 and 4 used here
> > > which might be a problem.
> > > 
> > > See update_alias_info_with_stack_vars and uses of decls_to_pointers,
> > > in particular from set_mem_attributes_minus_bitpos where we preserve
> > > TBAA info with the rewrite.  I'm not sure why that should be OK ...
> > > (but I'm sure I must have thought of this problem back in time)
> > > 
> > > Does the following fix the testcase?
> > > 
> > > diff --git a/gcc/emit-rtl.cc b/gcc/emit-rtl.cc
> > > index 84b6833225e..81c0a63eddc 100644
> > > --- a/gcc/emit-rtl.cc
> > > +++ b/gcc/emit-rtl.cc
> > > @@ -2128,7 +2128,7 @@ set_mem_attributes_minus_bitpos (rtx ref, tree t, int
> > > objectp,
> > >               tree *orig_base = &attrs.expr;
> > >               while (handled_component_p (*orig_base))
> > >                 orig_base = &TREE_OPERAND (*orig_base, 0);
> > > -             tree aptrt = reference_alias_ptr_type (*orig_base);
> > > +             tree aptrt = ptr_type_node;
> > >               *orig_base = build2 (MEM_REF, TREE_TYPE (*orig_base), *namep,
> > >                                    build_int_cst (aptrt, 0));
> > >             }
> > 
> > Sorry, this doesn't help.
> > 
> > I noticed that it makes insns 384 and 389 become to:
> > 
> > (insn 384 383 0 (set (mem/c:V2DI (plus:DI (reg/f:DI 112 virtual-stack-vars)
> >                 (const_int 16 [0x10])) [7 MEM <struct Vec128D.30433>
> > [(voidD.48 *)_10]+0 S16 A128])
> >         (reg:V2DI 616)) -1
> >      (nil))
> > 
> > (insn 389 388 390 (set (reg:HI 619)
> >         (mem/c:HI (plus:DI (reg/f:DI 112 virtual-stack-vars)
> >                 (const_int 16 [0x10])) [4 MEM <struct Vec128D.30212>
> > [(voidD.48 *)_10].rawD.30221[0]+0 S2 A128])) "test.cc":218:14 -1
> >      (nil))
> > 
> > alias sets are not changed.
> 
> Ah, probably the alias-set is determined from the unmangled ref ...
> 
> > Aggressively further hacking with attrs.alias =
> > 0 can make it pass. Can we make an new alias set for each partition? then
> > all involved decls in the same partition is aliased. For a particular
> > involved decl, it's aliased to the previous ones and the new ones in its own
> > partitions.
> 
> hmm, no - this won't work.  In fact even attrs.alias = 0 will probably
> not work reliably since we can coalesce variables that escape and thus
> the above will only alter accesses via the original decls but not any
> accesses done via pointers.  So indeed any alias-set mangling is pointless
> here.
> 
> Consider
> 
>  {
>    A x;
>    int * volatile p = &x;
>    *p = 1;
>    .. = *p;
>  }
>  {
>    B y;
>    float * volatile q = &y;
>    *q = 1;
>    .. = *q;
>  }
> 
> if we coalesce x and y then we are not rewriting any accesses
> but obviously the accesses still need to conflict - but the
> indirect accesses will have their original non-conflicting alias-set
> and thus the scheduler would be free to move the store to *q across
> the load from *p (the "trick" would be to make an incentive to do so
> of course).

Thanks for the clarification! Is it possible to update the alias set for the
indirect accesses as well? since we know the address is originally taken from
one coalesced decl (also update its propagated ones).