On Wed, Jun 17, 2026 at 4:44 PM <[email protected]> wrote: > > 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.
Ugh. So do those SSA variables have BLKmode? IMO the error then is really out-of-SSA which should have assigned distinct decls to overlapping partitions. That said, it's bad that we have to drop MEM_EXPRs but until we improve the above it looks like an OK fix, but ... > > 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)) this should not happen ... > + 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); how do we expand anonymous SSA vars (!var) here? You could try with a GIMPLE testcase, dropping the decl. Can we avoid setting DECL_RTL in cfgexpand.cc:set_rtl? It seems all information is already there? > + } > + > /* 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) >
