https://gcc.gnu.org/bugzilla/show_bug.cgi?id=125683
--- Comment #4 from GCC Commits <cvs-commit at gcc dot gnu.org> --- The master branch has been updated by Jeff Law <[email protected]>: https://gcc.gnu.org/g:542d7eaf767057fbd224d44dbc1772dea3e41891 commit r17-3250-g542d7eaf767057fbd224d44dbc1772dea3e41891 Author: Rohith Kapelli <[email protected]> Date: Wed Aug 12 22:05:23 2026 -0600 [PATCH v3] simplify-rtx: widen the memory attributes when folding "c ? a : a" [PR125683] 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. --- Changes since v2: MEM_READONLY_P, MEM_NOTRAP_P and MEM_POINTER are now dropped when the two arms disagree. They are rtx flag bits rather than MEM_ATTRS fields, so shallow_copy_rtx brings them across from the first operand and they have to be cleared by hand; they are cleared on the fresh MEM the fold already builds, not on either operand, which can be shared. The equal-attributes early exit tests them too, so a disagreement in a flag alone still goes through the copy. merge_memattrs already clears the first two this way, so MEM_POINTER is the one flag this adds to that treatment. New test gcc.dg/rtl/aarch64/pr125683-flags.c builds the disagreement directly, because it cannot be produced from C source: MEM_READONLY_P and MEM_NOTRAP_P are derived from the base object, which two loads from the same address share. It is gated to aarch64-*-* because rtl.exp collects tests recursively, so every __RTL test has to gate itself, and __RTL tests hard-code target register names. Testing, on aarch64-unknown-linux-gnu: patched and unpatched bootstrapped from clean trees in the same environment, both stage2 == stage3. Comparing every result line across gcc, g++, libstdc++, libgomp, libitm and libatomic -- normalising the build directory, which g++.dg/modules embeds in test names -- the two runs are identical except for this patch's own tests. One apparent transition, g++.dg/tsan/pthread_cond_clockwait.C at -O0, is a flaky execution test: re-run idle eight times per compiler it passed 2/8 unpatched and 1/8 patched. The runtime test is revert-sensitive: the unpatched compiler aborts on it, the patched one exits 0. The patch applies cleanly to trunk and to the gcc-16, gcc-15, gcc-14 and gcc-13 branches. 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]>
