Re: [PATCH v3] simplify-rtx: widen the memory attributes when folding "c ? a : a" [PR125683]

On 8/8/2026 8:54 AM, Rohith Kapelli wrote:
> simplify_ternary_operation folds IF_THEN_ELSE (cond, a, a) to a using
> rtx_equal_p, which ignores the memory attributes.  Two loads from the
> same address into the same register are rtx_equal_p even when they have
> incompatible alias sets, so the fold returned one arm's MEM and the
> result inherited just that arm's (too narrow) alias set.
>
> noce_try_ifelse_collapse builds exactly such an IF_THEN_ELSE from the two
> sides of
>
>    long f (int a, void *cc, long *d)
>    {
>      long long c;
>      *d = 0;
>      if (a) c = *(long *) cc; else c = *(long long *) cc;
>      *d = 1;
>      return c;
>    }
>
> so ce1 replaced the two loads with a single long long load; a later pass,
> seeing that a long long load does not alias the long store to *d, deleted
> the "dead" *d = 0, and with cc == d the function returned a stale value.
>
> Rather than dropping the fold when the attributes differ, keep it and
> return a reference that only claims what both operands guarantee, in the
> spirit of merge_memattrs: alias set 0 when the sets differ, MEM_EXPR and
> offset cleared when they disagree, and the minimum alignment.  The
> operands can be shared, so the attributes are set on a shallow copy
> rather than in place.
>
> MEM_READONLY_P, MEM_NOTRAP_P and MEM_POINTER each assert something about
> the reference, so the copy keeps them only when both operands do.  They
> are rtx flag bits rather than MEM_ATTRS fields, so shallow_copy_rtx takes
> them from the first operand and they have to be cleared by hand; the
> equal-attributes early exit tests them too, so a disagreement in a flag
> alone still goes through the copy.  merge_memattrs already drops the
> first two this way when it commons two references, so this only follows
> it; it does not look at MEM_POINTER, which is treated the same way here
> because it is an assertion about the loaded value in just the same
> sense.
>
> Unlike merge_memattrs, which fixes up two references that both remain in
> the instruction stream, this returns a single reference standing in for
> either arm, so the size is kept only when both agree instead of taking
> the larger one.  BLKmode is left alone because there MEM_ATTRS describes
> the size of the access itself.  Volatility is not merged: it constrains
> when the access happens rather than describing the memory, so it can be
> neither weakened (that would lose a required access) nor strengthened as
> merge_memattrs does (that would add a volatile access on the arm which
> did not have one, there being a single access left for either arm), and
> the fold is declined instead.  Address spaces need no check: rtx_equal_p
> already fails for MEMs in different address spaces.
>
> Several tree passes can factor the two loads with a conservative type
> before RTL and so hide this: PRE and code hoisting on the release
> branches, and the phi-opt load factoring (PR125557) on trunk.  The test
> disables them so the if-conversion path is exercised on every affected
> version.  It is a live wrong-code at -O2 on the 13/14/15/16 branches --
> gcc-13 miscompiles the reduced case at plain -O2 with no such flags at
> all.
>
> 	PR rtl-optimization/125683
>
> gcc/ChangeLog:
>
> 	* simplify-rtx.cc (simplify_context::simplify_ternary_operation):
> 	When folding an IF_THEN_ELSE of two equal MEM operands with
> 	different memory attributes, return a copy whose attributes are
> 	widened to what both operands allow.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/pr125683.c: New test.
> 	* gcc.dg/rtl/aarch64/pr125683-flags.c: New test.
>
> Signed-off-by: Rohith Kapelli<[email protected]>
Thanks.  I've pushed this to the trunk.  I'll backport to the 
appropriate release branches after it's had time to simmer on the trunk.

Jeff