[Bug rtl-optimization/125683] [13/14/15/16/17 Regression] wrong code due to ce1 combining 2 loads of different incompatible aliasing sets

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]>