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)
