From: Kyrylo Tkachov <[email protected]> A gimple register that cannot be kept in a hard register (for example an oversized vector such as V16DI on AArch64, which has no register mode) may have several simultaneously live SSA partitions. When such partitions are spilled, each is given its own distinct stack slot, yet set_rtl, via set_mem_attributes, attaches the base VAR_DECL of the SSA name as the MEM_EXPR of every slot, at offset 0 and with the size of the whole variable. The distinct slots therefore all claim to be the same object.
This misleads MEM_EXPR based disambiguation. In particular the load/store pair fusion pass groups candidate accesses by (decl, offset) (track_via_mem_expr), so a store to one slot at offset K and a store to a different slot at offset K +/- access_size are treated as adjacent and fused, redirecting an access to the wrong slot and producing wrong code. This shows up as a miscompile at -O2/-O3 on AArch64 (PR121957, where late-combine first reshapes the addressing into the affected form, and PR123625). Both the early (pre-RA) and late (post-RA) copies of the pass are affected. A stack slot assigned to an SSA partition represents a single coalesced SSA live-range, not the storage of the user variable, so attaching the variable as its MEM_EXPR is not meaningful, and is positively wrong once the variable occupies more than one slot. Fix this in pass_expand::execute: after every SSA partition has been given its final rtl, drop the MEM_EXPR of the stack slots of any decl that lives in more than one place. Such a decl is exactly the one whose DECL_RTL set_rtl marked with pc_rtx, so we only touch the slots that actually share a misleading MEM_EXPR; single-slot objects are left unchanged. The distinct RTL base addresses of the slots continue to disambiguate them, and in-slot accesses, which share a base register, are still paired normally, so ldp/stp formation for ordinary code is unaffected. The clear is done after the partition-adjustment loop rather than in set_rtl itself: set_rtl is reached more than once per slot (notably from adjust_one_expanded_partition_var), and clearing the MEM_EXPR mid-way would make a later set_rtl re-run set_mem_attributes after DECL_RTL has been set, tripping the assertion in set_mem_attributes_minus_bitpos. This is a broader fix that makes https://gcc.gnu.org/pipermail/gcc-patches/2026-June/720419.html redundant, so I'm proposing we drop that patch and go with this. Bootstrapped and tested on aarch64-none-linux-gnu and x86_64-linux. Ok for trunk? Thanks, Kyrill Signed-off-by: Kyrylo Tkachov <[email protected]> gcc/ChangeLog: PR target/123625 PR target/121957 * cfgexpand.cc (pass_expand::execute): After expanding SSA partitions, drop the MEM_EXPR of the stack slots of any decl that 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 | 26 ++++++++++ .../gcc.c-torture/execute/pr121957.c | 27 +++++++++++ .../gcc.c-torture/execute/pr123625.c | 48 +++++++++++++++++++ 3 files changed, 101 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..76c2dbe7a29 100644 --- a/gcc/cfgexpand.cc +++ b/gcc/cfgexpand.cc @@ -7167,6 +7167,32 @@ pass_expand::execute (function *fun) adjust_one_expanded_partition_var (name); } + /* A gimple register that cannot live in a hard register (e.g. an oversized + vector with no register mode) may be split into several simultaneously + live SSA partitions, each given its own distinct stack slot. set_rtl, via + set_mem_attributes, attaches the base decl as the MEM_EXPR of every such + slot at offset 0, so the distinct slots all claim to be the same object. + That misleads MEM_EXPR-based disambiguation: the load/store pair-fusion + pass keys candidate accesses on (decl, offset) and would treat accesses to + different slots as adjacent, fusing them and corrupting one slot (see + PR123625, PR121957). A decl that lives in more than one place is exactly + the one whose DECL_RTL set_rtl marked with pc_rtx; drop the bogus MEM_EXPR + on its stack slots. The slots' distinct RTL base addresses continue to + disambiguate them correctly. This must run after the loop above, once all + set_rtl calls (which would otherwise re-attach the MEM_EXPR) are done. */ + FOR_EACH_SSA_NAME (i, name, cfun) + { + if (!SSA_NAME_DEF_STMT (name)) + continue; + int part = var_to_partition (SA.map, name); + if (part == NO_PARTITION) + continue; + tree var = SSA_NAME_VAR (name); + rtx x = SA.partition_to_pseudo[part]; + if (var && DECL_RTL_IF_SET (var) == pc_rtx && x && MEM_P (x)) + 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..cfe5fa468aa --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr121957.c @@ -0,0 +1,27 @@ +/* PR target/121957: 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..0dae287d227 --- /dev/null +++ b/gcc/testsuite/gcc.c-torture/execute/pr123625.c @@ -0,0 +1,48 @@ +/* PR target/123625: 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)
