From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 48) id 8BDE63858C5F; Mon, 23 Oct 2023 03:21:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8BDE63858C5F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1698031280; bh=NNA1dPFWh/LmywVDLmjmsYcWTaxAhzPsXt17tkYRwpg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=GoLGN33gDdiQKmgWy8PAn2OvQjk7wtV84raCJVSIHjKtwg/7Y7g3heV/XOZqFfTZl yYlNe80tp7IgS+u0G9E881rsonDp44cmpnya3lDU5v8qI/jmPnU7KcC8ICuORfQCw7 nJeRgIKBPjlvLvLxPNF7GSuE4rIMg1sjziyyikx8= From: "linkw at gcc dot gnu.org" To: gcc-bugs@gcc.gnu.org Subject: [Bug target/111591] ppc64be: miscompilation with -mstrict-align / -O3 Date: Mon, 23 Oct 2023 03:21:18 +0000 X-Bugzilla-Reason: CC X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: gcc X-Bugzilla-Component: target X-Bugzilla-Version: 13.2.0 X-Bugzilla-Keywords: needs-bisection X-Bugzilla-Severity: normal X-Bugzilla-Who: linkw at gcc dot gnu.org X-Bugzilla-Status: ASSIGNED X-Bugzilla-Resolution: X-Bugzilla-Priority: P3 X-Bugzilla-Assigned-To: linkw at gcc dot gnu.org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 List-Id: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D111591 --- Comment #26 from Kewen Lin --- (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: > > >=20 > > > (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)) > > >=20 > > > vs. > > >=20 > > > (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)) > > >=20 > > > both are based off a fake _10. But we get alias sets 7 and 4 used he= re > > > which might be a problem. > > >=20 > > > 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) > > >=20 > > > Does the following fix the testcase? > > >=20 > > > 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 =3D &attrs.expr; > > > while (handled_component_p (*orig_base)) > > > orig_base =3D &TREE_OPERAND (*orig_base, 0); > > > - tree aptrt =3D reference_alias_ptr_type (*orig_base); > > > + tree aptrt =3D ptr_type_node; > > > *orig_base =3D build2 (MEM_REF, TREE_TYPE (*orig_base),= *namep, > > > build_int_cst (aptrt, 0)); > > > } > >=20 > > Sorry, this doesn't help. > >=20 > > I noticed that it makes insns 384 and 389 become to: > >=20 > > (insn 384 383 0 (set (mem/c:V2DI (plus:DI (reg/f:DI 112 virtual-stack-v= ars) > > (const_int 16 [0x10])) [7 MEM > > [(voidD.48 *)_10]+0 S16 A128]) > > (reg:V2DI 616)) -1 > > (nil)) > >=20 > > (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 > > [(voidD.48 *)_10].rawD.30221[0]+0 S2 A128])) "test.cc":218:14 -1 > > (nil)) > >=20 > > alias sets are not changed. >=20 > Ah, probably the alias-set is determined from the unmangled ref ... >=20 > > Aggressively further hacking with attrs.alias =3D > > 0 can make it pass. Can we make an new alias set for each partition? th= en > > 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 it= s own > > partitions. >=20 > hmm, no - this won't work. In fact even attrs.alias =3D 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. >=20 > Consider >=20 > { > A x; > int * volatile p =3D &x; > *p =3D 1; > .. =3D *p; > } > { > B y; > float * volatile q =3D &y; > *q =3D 1; > .. =3D *q; > } >=20 > 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 fr= om one coalesced decl (also update its propagated ones).=