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

On 7/27/2026 10:44 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.
>
> 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, and the fold is also skipped when the two
> operands disagree about volatility.  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.
>
> Signed-off-by: Rohith Kapelli <[email protected]>
This version is better.  I think we probably have to worry about 
MEM_READONLY_P.   For the transformation in question if only one of the 
two loads has MEM_READONLY_P, then we would drop that property on the 
if-converted load.  I can't immediately think of when that could happen, 
but I'm willing to believe it could possibly happen through one or more 
obscure transformations.


We should check MEM_NOTRAP_P and MEM_POINTER as well.  I think in both 
cases if there is disagreement, then we drop the flag.

I think with those fixes in place we'll be ready to integrate.

jeff