Adding the CC’s from the previous version of the patch submission. Sorry for missing them out initially.
> On 9 Jul 2026, at 14:51, Kyrylo Tkachov <[email protected]> wrote: > > From: Kyrylo Tkachov <[email protected]> > > Changes in v2: > - Match the exact expansion spill predicate instead of using BLKmode. > - Add a non-BLKmode -ffloat-store regression. > > Out-of-SSA can leave several partitions sharing one base VAR_DECL when its > SSA versions have overlapping live ranges. If those partitions are spilled, > set_mem_attributes gives every stack slot the same MEM_EXPR at offset zero. > MEM_EXPR-based disambiguation can then combine accesses from distinct slots. > > Split the overlapping partitions after TER, using the same > use_register_for_decl predicate as expansion rather than approximating the > memory decision with BLKmode. Keep the default-definition partition tied to > the original decl and give the other partitions artificial decls with debug > expressions referring back to it. > > Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux. > > gcc/ChangeLog: > > PR target/123625 > PR target/121957 > * tree-outof-ssa.cc (split_overlapping_partition_decls): New function. > Use use_register_for_decl to identify memory partitions. > (remove_ssa_form): Call split_overlapping_partition_decls. > > gcc/testsuite/ChangeLog: > > PR target/123625 > PR target/121957 > * gcc.c-torture/execute/pr121957.c: New test. > * gcc.c-torture/execute/pr123625.c: New test. > * gcc.c-torture/execute/pr123625-2.c: New test. > * gcc.c-torture/execute/pr123625-3.c: New test. > * gcc.dg/pr123625-1.c: New test. > > Signed-off-by: Kyrylo Tkachov <[email protected]> > --- > .../gcc.c-torture/execute/pr121957.c | 27 ++++++ > .../gcc.c-torture/execute/pr123625-2.c | 34 ++++++++ > .../gcc.c-torture/execute/pr123625-3.c | 32 +++++++ > .../gcc.c-torture/execute/pr123625.c | 48 +++++++++++ > gcc/testsuite/gcc.dg/pr123625-1.c | 22 +++++ > gcc/tree-outof-ssa.cc | 86 +++++++++++++++++++ > 6 files changed, 249 insertions(+) > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr121957.c > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr123625-2.c > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr123625-3.c > create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr123625.c > create mode 100644 gcc/testsuite/gcc.dg/pr123625-1.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-2.c > b/gcc/testsuite/gcc.c-torture/execute/pr123625-2.c > new file mode 100644 > index 00000000000..50ee842519c > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr123625-2.c > @@ -0,0 +1,34 @@ > +/* With -ftree-coalesce-vars, one variable can have SSA names coalesced into > + partitions of other variables: the PHIs below put the two names of b into > + the partitions of a and of c. All three variables are oversized vectors > + with no register mode, so both partitions are spilled and b legitimately > + lives in two distinct stack slots (its DECL_RTL becomes the "multiple > + places" marker). Out-of-SSA must keep the two slots distinguishable > + without rejecting this state. */ > + > +typedef long __attribute__((vector_size (16 * sizeof (long)))) v16di; > + > +v16di g0 = { 1 }, g1 = { 5 }, g2 = { 3 }, g3 = { 7 }; > +volatile int p, q; > + > +int > +main (void) > +{ > + v16di b = g0; > + v16di a; > + if (p) > + a = b; > + else > + a = g1; > + g1 = a; > + b = g2; > + v16di c; > + if (q) > + c = b; > + else > + c = g3; > + g3 = c; > + if (g1[0] != 5 || g1[1] != 0 || g3[0] != 7 || g3[1] != 0) > + __builtin_abort (); > + return 0; > +} > diff --git a/gcc/testsuite/gcc.c-torture/execute/pr123625-3.c > b/gcc/testsuite/gcc.c-torture/execute/pr123625-3.c > new file mode 100644 > index 00000000000..ccac48f7797 > --- /dev/null > +++ b/gcc/testsuite/gcc.c-torture/execute/pr123625-3.c > @@ -0,0 +1,32 @@ > +/* Same state as pr123625-2.c (one variable coalesced into two other > + variables' spilled partitions) reached through asm goto control flow. */ > + > +typedef long __attribute__((vector_size (16 * sizeof (long)))) v16di; > + > +v16di g0, g1, g2, g3; > + > +int > +main (void) > +{ > + v16di b = g0; > + v16di a; > + asm goto ("" : : : : L0); > + a = b; > + goto L1; > +L0: > + a = g1; > +L1: > + g1 = a; > + b = g2; > + v16di c; > + asm goto ("" : : : : L2); > + c = b; > + goto L3; > +L2: > + c = g3; > +L3: > + g3 = c; > + if (g1[0] | g1[1] | g3[0] | g3[1]) > + __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/testsuite/gcc.dg/pr123625-1.c > b/gcc/testsuite/gcc.dg/pr123625-1.c > new file mode 100644 > index 00000000000..f88273e95c6 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/pr123625-1.c > @@ -0,0 +1,22 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fgimple -ffloat-store -ftree-coalesce-vars > -fdump-rtl-expand" } */ > + > +double in1, in2, out1, out2; > + > +void __GIMPLE (ssa, startwith ("expand")) > +f (void) > +{ > + double v; > + > +__BB(2): > + v_1 = in1; > + v_2 = in2; > + out1 = v_1; > + out2 = v_2; > + return; > +} > + > +/* The scalar floating-point SSA names overlap, and -ffloat-store assigns > + both partitions to memory. */ > +/* { dg-final { scan-rtl-dump {\[[0-9]+ v\+0} "expand" } } */ > +/* { dg-final { scan-rtl-dump {\[[0-9]+ D\.[0-9]+\+0} "expand" } } */ > diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc > index f4bc4878bd6..918dbbedde3 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,86 @@ 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 but one 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 partitions that will live in memory can end up with a > + misleading shared MEM_EXPR. Mirror the decision that > + expand_one_ssa_partition will make. */ > + if (use_register_for_decl (repr)) > + continue; > + /* One partition of VAR keeps the user decl, the rest are split. > + A default definition cannot change its variable, so if VAR has a > + partitioned default definition, its partition is the one that > + keeps the user decl. Otherwise the first partition seen does. */ > + tree ddef = ssa_default_def (cfun, var); > + int keep = ddef ? var_to_partition (map, ddef) : NO_PARTITION; > + if (keep == NO_PARTITION) > + { > + if (!seen.add (var)) > + continue; > + } > + else if (keep >= 0 && (unsigned) keep == i) > + 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 +1161,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) >
