Ping on this or the alternative at: https://gcc.gnu.org/pipermail/gcc-patches/2026-June/721973.html
As this is a wrong-code bug I’d like to get something in for the 16.2 release. Thanks, Kyrill > On 30 Jun 2026, at 11:01, Kyrylo Tkachov <[email protected]> wrote: > > From: Kyrylo Tkachov <[email protected]> > > A gimple register with no register mode (e.g. an oversized vector such as > V16DI > on AArch64, which is BLKmode) may have several simultaneously live SSA > versions > that out-of-SSA cannot coalesce, so they end up in separate partitions sharing > one base VAR_DECL. When such partitions are spilled they are given distinct > stack slots, but set_mem_attributes gives every slot that one decl as its > MEM_EXPR at offset 0, so the slots all appear to be the same object. > > This misleads MEM_EXPR-based disambiguation: the load/store pair-fusion pass > keys candidate accesses on (decl, offset) and fuses stores across the slots, > corrupting one and producing wrong code at -O2/-O3 (PR123625, and PR121957 > where late-combine first reshapes the addressing into this form). > > This is an alternative fix to the expand one posted at: > https://gcc.gnu.org/pipermail/gcc-patches/2026-June/721973.html > > Fix it in remove_ssa_form after coalescing, give every partition > after the first of such a BLKmode decl its own artificial decl, so the slots > are distinguished. The new decl carries a DECL_DEBUG_EXPR back to the user > variable so debug info still attributes the storage to it (as > create_access_replacement does in tree-sra.cc). PARM_DECLs and RESULT_DECLs > are left alone. They require a single partition holding the canonical RTL. > The > split runs after find_replaceable_exprs so it does not perturb TER. > > Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux. > Is this preferable to the expand fix? > Thanks, > Kyrill > > gcc/ChangeLog: > > PR target/123625 > PR target/121957 > * tree-outof-ssa.cc (split_overlapping_partition_decls): New function. > (remove_ssa_form): Call it. > > gcc/testsuite/ChangeLog: > > PR target/123625 > PR target/121957 > * gcc.c-torture/execute/pr123625.c: New test. > * gcc.c-torture/execute/pr121957.c: New test. > > Signed-off-by: Kyrylo Tkachov <[email protected]> > --- > .../gcc.c-torture/execute/pr121957.c | 27 +++++++ > .../gcc.c-torture/execute/pr123625.c | 48 ++++++++++++ > gcc/tree-outof-ssa.cc | 75 +++++++++++++++++++ > 3 files changed, 150 insertions(+) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr121957.c > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr123625.c > > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr121957.c > b/gcc/testsuite/gcc.c-torture/execute/pr121957.c > new file mode 100644 > index 00000000000..98969566c29 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr121957.c > @@ -0,0 +1,27 @@ > +/* AArch64 wrong code at -O2/-O3. An oversized vector > + (V16DI, 128 bytes, with no register mode) is expanded into two distinct > + stack slots that share the same MEM_EXPR. The load/store pair-fusion pass > + then discovers "adjacent" store pairs across the two slots via the shared > + MEM_EXPR and fuses them, redirecting a store to the wrong slot and leaving > + part of a slot uninitialised. Self-checking: aborts if the result is > + wrong. */ > + > +typedef long __attribute__((vector_size (16 * sizeof (long)))) v16di; > + > +int > +main (void) > +{ > + v16di v = {}; > + asm goto ("" : : : : L1); > +L2: > + asm goto ("" : : : : L1); > +L0: > + asm goto ("" : : : : L2); > + v = (v16di){ -1 }; > + asm goto ("" : : : : L0); > +L1: > + asm goto ("" : : : : L0); > + if (v[3]) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr123625.c > b/gcc/testsuite/gcc.c-torture/execute/pr123625.c > new file mode 100644 > index 00000000000..2b3b29b5e14 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr123625.c > @@ -0,0 +1,48 @@ > +/* AArch64 wrong code at -O3. An oversized vector > + (V16DI, 128 bytes, no register mode) is expanded into two distinct > + stack slots that shared the same MEM_EXPR. The load/store pair fusion > + pass then fused stores belonging to different slots, leaving part of a > + slot uninitialized. Self-checking: the checksum is target-independent. > */ > + > +#include <stdint.h> > + > +#define BS_VEC(type, num) type __attribute__((vector_size(num * > sizeof(type)))) > +uint64_t BS_CHECKSUM, g_284; > +struct U0 > +{ > + int16_t f0; > + int64_t f2; > +} g_205; > +int16_t g_8, func_2_BS_COND_1; > +uint8_t g_9[2][1]; > +volatile struct U0 g_121[1]; > +int64_t *g_565 = &g_284; > + > +int > +main (void) > +{ > + BS_VEC (int64_t, 16) BS_VAR_3 = { 1, 8096386231136, 9039249955151 }; > + uint64_t LOCAL_CHECKSUM = 0; > + switch (func_2_BS_COND_1) > + { > + case 2: goto BS_LABEL_0; > + case 4: goto BS_LABEL_0; > + } > + for (g_8 = 0; g_8 <= 0; g_8 += 1) > + for (g_205.f2 = 0; g_205.f2 <= 1; g_205.f2 += 1) > + { > + if (g_9[g_205.f2][0]) > + BS_LABEL_0: > + for (;;) > + ; > + for (uint32_t BS_TEMP_371 = 0; BS_TEMP_371 < 16; BS_TEMP_371++) > + LOCAL_CHECKSUM ^= BS_VAR_3[BS_TEMP_371] + 9 > + + (LOCAL_CHECKSUM << 6) + LOCAL_CHECKSUM > + >> (*g_565 |= g_121[0].f0); > + BS_VAR_3 = (BS_VEC (int64_t, 16)){}; > + } > + BS_CHECKSUM = LOCAL_CHECKSUM; > + if (BS_CHECKSUM != 0x7237237237237237ULL) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc > index f4bc4878bd6..78372d314c0 100644 > --- a/gcc/tree-outof-ssa.cc > +++ b/gcc/tree-outof-ssa.cc > @@ -25,6 +25,7 @@ along with GCC; see the file COPYING3. If not see > #include "rtl.h" > #include "tree.h" > #include "gimple.h" > +#include "gimple-expr.h" > #include "cfghooks.h" > #include "ssa.h" > #include "tree-ssa.h" > @@ -1049,6 +1050,75 @@ expand_phi_nodes (struct ssaexpand *sa) > } > > > +/* Out-of-SSA can leave several partitions sharing one base VAR_DECL, when > + that variable's SSA versions are simultaneously live and so cannot all be > + coalesced. When such a partition is spilled (it has no register mode, > + e.g. an oversized vector that is BLKmode), set_mem_attributes would give > + every one of those slots that single decl as its MEM_EXPR at offset 0, so > + the distinct slots appear to be the same object and mislead MEM_EXPR-based > + disambiguation, and the load/store pair-fusion pass then fuses across the > + slots and corrupts one. Give every partition after the first of such a > + decl its own artificial decl so the slots are distinguished at the source. > + The new decl carries a DECL_DEBUG_EXPR back to the user variable so debug > + info still attributes the storage to it (cf. create_access_replacement in > + tree-sra.cc). PARM_DECLs and RESULT_DECLs are left alone, as they require > + a single partition holding the canonical RTL. */ > + > +static void > +split_overlapping_partition_decls (var_map map) > +{ > + unsigned n = num_var_partitions (map); > + hash_set<tree> seen; > + auto_vec<tree> new_decl; > + new_decl.safe_grow_cleared (n); > + bool any = false; > + > + for (unsigned i = 0; i < n; i++) > + { > + tree repr = partition_to_var (map, i); > + if (!repr) > + continue; > + tree var = SSA_NAME_VAR (repr); > + if (!var || !VAR_P (var)) > + continue; > + /* Only memory partitions (no register mode, e.g. an oversized vector) > + can end up with a misleading shared MEM_EXPR. */ > + if (TYPE_MODE (TREE_TYPE (var)) != BLKmode) > + continue; > + /* The first partition of VAR keeps the user decl, the rest are split. > */ > + if (!seen.add (var)) > + continue; > + > + tree nvar = create_tmp_var_raw (TREE_TYPE (var)); > + DECL_CONTEXT (nvar) = DECL_CONTEXT (var); > + DECL_SOURCE_LOCATION (nvar) = DECL_SOURCE_LOCATION (var); > + SET_DECL_ALIGN (nvar, DECL_ALIGN (var)); > + if (!DECL_ARTIFICIAL (var) && DECL_NAME (var)) > + { > + SET_DECL_DEBUG_EXPR (nvar, var); > + DECL_HAS_DEBUG_EXPR_P (nvar) = 1; > + } > + copy_warning (nvar, var); > + add_local_decl (cfun, nvar); > + new_decl[i] = nvar; > + any = true; > + } > + > + if (!any) > + return; > + > + unsigned ver; > + tree name; > + FOR_EACH_SSA_NAME (ver, name, cfun) > + { > + if (SSA_NAME_IS_DEFAULT_DEF (name)) > + continue; > + int p = var_to_partition (map, name); > + if (p != NO_PARTITION && new_decl[p]) > + SET_SSA_NAME_VAR_OR_IDENTIFIER (name, new_decl[p]); > + } > +} > + > /* Remove the ssa-names in the current function and translate them into normal > compiler variables. PERFORM_TER is true if Temporary Expression > Replacement > should also be used. */ > @@ -1080,6 +1150,11 @@ remove_ssa_form (bool perform_ter, struct ssaexpand > *sa) > dump_replaceable_exprs (dump_file, values); > } > > + /* Distinct partitions of one decl must not share a MEM_EXPR once they are > + spilled to separate stack slots. Done after TER so reassigning > + SSA_NAME_VAR does not perturb find_replaceable_exprs. */ > + split_overlapping_partition_decls (map); > + > rewrite_trees (map); > > sa->map = map; > -- > 2.50.1 (Apple Git-155) >
