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
partitions, each spilled to its own stack slot. set_rtl, via
set_mem_attributes, gives every such slot the base decl as its MEM_EXPR at
offset 0, so the distinct slots all claim to be the same object.
This misleads MEM_EXPR-based disambiguation. The load/store pair-fusion pass
keys candidate accesses on (decl, offset) (track_via_mem_expr), so stores to
different slots at the same offset are treated as adjacent and fused,
redirecting an access to the wrong slot and producing wrong code at -O2/-O3
(PR123625, and PR121957 where late-combine first reshapes the addressing into
this form).
Fix it in pass_expand::execute: once all partitions have rtl, drop the MEM_EXPR
on the stack slot of any partition whose base decl lives in more than one place
(exactly the decls set_rtl marked with pc_rtx). Single-slot objects are
untouched, and the slots' distinct base addresses still disambiguate them, so
ordinary ldp/stp formation is unaffected.
Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux.
gcc/ChangeLog:
PR target/123625
PR target/121957
* cfgexpand.cc (pass_expand::execute): After expanding SSA
partitions, drop the MEM_EXPR of the stack slot of any partition
whose base decl occupies more than one partition.
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/cfgexpand.cc | 21 ++++++++
.../gcc.c-torture/execute/pr121957.c | 27 +++++++++++
.../gcc.c-torture/execute/pr123625.c | 48 +++++++++++++++++++
3 files changed, 96 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/cfgexpand.cc b/gcc/cfgexpand.cc
index 05efcea13ba..ee31f05fa83 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -7167,6 +7167,27 @@ pass_expand::execute (function *fun)
adjust_one_expanded_partition_var (name);
}
+ /* A gimple register with no register mode (e.g. an oversized vector such as
+ V16DI on AArch64, which is BLKmode) can be spilled to several distinct
+ stack slots, one per simultaneously-live SSA partition. set_rtl gives
+ each slot the base decl as its MEM_EXPR at offset 0, so the slots all
+ alias as the same object and the load/store pair-fusion pass, which keys
+ candidates on (decl, offset), may fuse stores across them and corrupt one
+ slot. Such a decl lives in more than one place, i.e. set_rtl marked
+ its DECL_RTL with pc_rtx. Drop the bogus MEM_EXPR on those
+ slots. Do it here, once all partitions have rtl and before the loop below
+ clears the pc_rtx marker. */
+ for (unsigned p = 0; p < num_var_partitions (SA.map); p++)
+ {
+ rtx x = SA.partition_to_pseudo[p];
+ if (!x || !MEM_P (x))
+ continue;
+ tree repr = partition_to_var (SA.map, p);
+ tree var = repr ? SSA_NAME_VAR (repr) : NULL_TREE;
+ if (var && DECL_RTL_IF_SET (var) == pc_rtx)
+ set_mem_expr (x, NULL_TREE);
+ }
+
/* Clean up RTL of variables that straddle across multiple
partitions, and check that the rtl of any PARM_DECLs that are not
cleaned up is that of their default defs. */
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;
+}
--
2.50.1 (Apple Git-155)