Re: [PATCH 1/2] cfgexpand: Check partition MEM_EXPRs once the RTL of parameters is final [PR126701]

On Fri, 7 Aug 2026, [email protected] wrote:

> From: Kyrylo Tkachov <[email protected]>
> 
> verify_partition_mem_exprs checks that no two out-of-SSA partitions carry one
> MEM_EXPR at different addresses.  It ran before the loop that restores the RTL
> of PARM_DECL and RESULT_DECL default definitions, and that loop is what gives
> the partition of such a default definition its decl back:
> 
>   FOR_EACH_SSA_NAME (i, name, cfun)
>     adjust_one_expanded_partition_var (name);   /* Merges in a MEM_EXPR.  */
>   ...
>       if (MEM_P (in))
> 	set_mem_attributes (in, var, true);     /* Takes it out again.  */
> 
> Walking the names of a partition attaches the variable of any of them to that
> partition's location, preferring an ignored one.  A partition that holds the
> default definition of a parameter next to names of a store-motion temporary is
> therefore given the temporary as its MEM_EXPR for the span between the two
> loops, and the partition of the temporary itself has it as well:
> 
>   part 20  MEM_EXPR g0_lsm.14  addr (plus (virtual-stack-vars) (const_int -256))
>   part 26  MEM_EXPR g0_lsm.14  addr (reg/v:DI 142)
> 
> The parameter takes its MEM_EXPR back before any statement is expanded, so the
> insn stream is unaffected, but the check in between sees the two and aborts.
> 
> Run the check once the RTL of every partition is final.
> With the out-of-SSA fixes for PR126405 reverted it still fires on all
> three of that PR's tests, the PARM_DECL one included.
> 
> Bootstrapped and tested on aarch64-none-linux-gnu.
> Ok for trunk?

OK.

Thanks,
Richard.

> Thanks,
> Kyrill
> 
> gcc/ChangeLog:
> 
> 	PR middle-end/126701
> 	* cfgexpand.cc (verify_partition_mem_exprs): Document when it may
> 	be called.
> 	(pass_expand::execute): Call it after the RTL of PARM_DECL and
> 	RESULT_DECL default definitions is restored.
> 	* tree-outof-ssa.cc (split_overlapping_partition_decls): Note the
> 	variable a partition of a parameter or result default definition
> 	ends up with.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR middle-end/126701
> 	* gcc.dg/pr126701.c: New test.
> 
> Signed-off-by: Kyrylo Tkachov <[email protected]>
> ---
>  gcc/cfgexpand.cc                | 13 +++++--
>  gcc/testsuite/gcc.dg/pr126701.c | 67 +++++++++++++++++++++++++++++++++
>  gcc/tree-outof-ssa.cc           |  6 ++-
>  3 files changed, 81 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr126701.c
> 
> diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
> index 0b5bf7cd464..fd3f890620b 100644
> --- a/gcc/cfgexpand.cc
> +++ b/gcc/cfgexpand.cc
> @@ -2467,7 +2467,12 @@ stack_protect_return_slot_p ()
>     MEM_EXPR base and an offset from it, so two stack slots carrying one
>     MEM_EXPR read as a single object, which lets an access to one be redirected
>     to the other.  out-of-SSA keeps them apart, see the comment above
> -   split_overlapping_partition_decls.  */
> +   split_overlapping_partition_decls.
> +
> +   Call this once the RTL of every partition is final.  The partition of a
> +   PARM_DECL or RESULT_DECL default definition is given the MEM_EXPR of any
> +   other variable in it while its names are walked, and only gets the decl
> +   back when its RTL is restored in pass_expand::execute.  */
>  
>  static void
>  verify_partition_mem_exprs (void)
> @@ -7194,9 +7199,6 @@ pass_expand::execute (function *fun)
>        adjust_one_expanded_partition_var (name);
>      }
>  
> -  if (flag_checking)
> -    verify_partition_mem_exprs ();
> -
>    /* 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.  */
> @@ -7244,6 +7246,9 @@ pass_expand::execute (function *fun)
>  	}
>      }
>  
> +  if (flag_checking)
> +    verify_partition_mem_exprs ();
> +
>    /* If this function is `main', emit a call to `__main'
>       to run global initializers, etc.  */
>    if (DECL_NAME (current_function_decl)
> diff --git a/gcc/testsuite/gcc.dg/pr126701.c b/gcc/testsuite/gcc.dg/pr126701.c
> new file mode 100644
> index 00000000000..3f582cce5c3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr126701.c
> @@ -0,0 +1,67 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fchecking" } */
> +
> +/* The partition holding the default definition of A2 also holds names of the
> +   store-motion temporaries of G0.  Walking the names of that partition gives
> +   its RTL the MEM_EXPR of a temporary, which the partition of that temporary
> +   has too, but the parameter gets its own MEM_EXPR back before any statement
> +   is expanded.  */
> +
> +typedef unsigned long long v16u64 __attribute__((vector_size (128)));
> +
> +v16u64 g0;
> +_Bool g9, ob13;
> +unsigned char g17;
> +static unsigned char g24 = 3;
> +void *a3;
> +__attribute__((cold)) void f8 (_Bool, unsigned char);
> +
> +void
> +f30 (unsigned long long a0, _Bool a1, v16u64 a2)
> +{
> +  unsigned long long v8;
> +  _Bool c9;
> +cont1:
> +  a2 = g0;
> +  g0 = __builtin_shufflevector (g0, g0, 0, 1, 1, 4, 7, 2, 0, 8, 7, 7, 7, 8, 9,
> +				0, 1, 4);
> +  switch (a0)
> +    {
> +    case 201146615185186167:
> +      goto sw3;
> +    case 1:
> +      goto sw3;
> +    default:
> +      goto cont1;
> +    }
> +sw3:
> +  c9 = g24;
> +  goto bf66;
> +bf26:
> +  c9 = v8;
> +  a3 = &c9;
> +  if (c9)
> +    g0 = ~g0;
> +  if (a1)
> +    goto br51;
> +  goto bf66;
> +br32:
> +  if (ob13)
> +    goto br47;
> +  if (__builtin_expect_with_probability (g9, 0, 0.265))
> +    goto br32;
> +  goto bf66;
> +br47:
> +  f8 (a1, g17);
> +br51:
> +  a2 = g0;
> +  if (c9)
> +    goto bf26;
> +sw55:
> +  g0 = a2;
> +  return;
> +bf66:
> +  if (__builtin_expect_with_probability (c9, 0, 0.519))
> +    goto br32;
> +  goto sw55;
> +}
> diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc
> index af9d79fedcd..409e951b212 100644
> --- a/gcc/tree-outof-ssa.cc
> +++ b/gcc/tree-outof-ssa.cc
> @@ -1103,7 +1103,11 @@ split_overlapping_partition_decls (var_map map)
>        /* Expansion hands set_rtl the representative before the other names,
>  	 and expand_leader_merge keeps the variable it is given first unless a
>  	 later one is DECL_IGNORED_P, so merging the two gives the variable
> -	 this partition ends up with.  */
> +	 this partition ends up with.  A partition holding the default
> +	 definition of a parameter or of the result is instead seeded with that
> +	 decl, and is given it back once its RTL is restored at the end of
> +	 expansion, so the variable it ends up with is one that the rule below
> +	 keeps for it alone.  */
>        tree var = SSA_NAME_VAR (repr);
>        if (part_var[i])
>  	var = expand_leader_merge (var, part_var[i]);
> 

-- 
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)