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

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?
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]);
-- 
2.50.1 (Apple Git-155)