On Tue, 30 Jun 2026, [email protected] wrote:
> 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?
While I like this more I think the actual issue is in set_rtl.cc which
does strange things, but notably
tree next = skip ? cur : leader_merge (cur, SSAVAR (t) ? SSAVAR (t)
: t);
if (cur != next)
{
if (MEM_P (x))
set_mem_attributes (x,
next && TREE_CODE (next) == SSA_NAME
? TREE_TYPE (next)
: next, true);
that is, it is responsible for turning in SSA_NAME_VAR to
set_mem_attributes. It shouldn't do that if there are multiple
partitions for it. In fact I'm not sure why we ever do all this
singing and dancing.
IMO a simple fix(?) would be to drop all this and do
tree next = skip ? cur : leader_merge (cur, t);
esp. when MEM_P (x). There's similarly dubious code after this,
but we already note the issue!
if (TREE_CODE (t) == SSA_NAME)
{
int part = var_to_partition (SA.map, t);
if (part != NO_PARTITION)
{
if (SA.partition_to_pseudo[part])
gcc_assert (SA.partition_to_pseudo[part] == x);
else if (x != pc_rtx)
SA.partition_to_pseudo[part] = x;
}
/* For the benefit of debug information at -O0 (where
vartracking doesn't run) record the place also in the base
DECL. For PARMs and RESULTs, do so only when setting the
default def. */
if (x && x != pc_rtx && SSA_NAME_VAR (t)
&& (VAR_P (SSA_NAME_VAR (t))
|| SSA_NAME_IS_DEFAULT_DEF (t)))
{
tree var = SSA_NAME_VAR (t);
/* If we don't yet have something recorded, just record it now.
*/
if (!DECL_RTL_SET_P (var))
SET_DECL_RTL (var, x);
/* If we have it set already to "multiple places" don't
change this. */
else if (DECL_RTL (var) == pc_rtx)
;
/* If we have something recorded and it's not the same place
as we want to record now, we have multiple partitions for the
same base variable, with different places. We can't just
randomly chose one, hence we have to say that we don't know.
This only happens with optimization, and there var-tracking
will figure out the right thing. */
else if (DECL_RTL (var) != x)
SET_DECL_RTL (var, pc_rtx);
^^^^^^^^^^^^^^^^^^
what seems to be lacking is direct info of whether a particular
DECL has multiple SSA partitions.
Upthread we get here via expand_stack_vars -> expand_one_stack_var_at
where there is the stack_vars[] meta where I can see for one of the
testcases:
(gdb) p stack_vars[0]
$19 = {decl = <ssa_name 0x7ffff672dd10 3>, size = {coeffs = {128, 0}},
alignb = 16, representative = 0, next = 4294967295, conflicts = 0x0}
(gdb) p stack_vars[1]
$20 = {decl = <ssa_name 0x7ffff6779318 10>, size = {coeffs = {128, 0}},
alignb = 16, representative = 1, next = 4294967295, conflicts = 0x0}
(gdb) p debug_generic_expr (0x7ffff672dd10)
v_3
$21 = void
(gdb) p debug_generic_expr (0x7ffff6779318)
v_10
so there's two stack vars for the same SSA_NAME_VAR, exactly the
situation above. It should be possible to discover this case
beforehand and mark the affected stack_vars[] entries as to
skip using SSA_NAME_VAR here, possibly pass down the decl/type
to be used to set_rtl directly.
Your out-of-SSA patch might go for preserving as much info
as possible, so the last quoted piece above could be made
an assert that this doesn't happen after it? The only thing
is that keying on BLKmode might not be fully matching what
we do later. In principle the decl to be used could be
created in expand_stack_vars as well?
It was Alex who at least last touched all this and most of
set_rtl is attributed to ...
Thanks,
Richard.
> 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;
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald; (HRB 36809, AG Nuernberg)
