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)

Reply via email to