From: Kyrylo Tkachov <[email protected]>
split_overlapping_partition_decls gives every partition but one of a
memory-resident VAR_DECL its own artificial decl, so that distinct stack slots
do not end up sharing a MEM_EXPR. It takes the variable of a partition from
that partition's representative. set_rtl attaches the base variable of any
name in a partition to that partition's location, so the variable a partition
is given need not be the one of its representative.
Here the partition holding the loop-carried versions of an oversized vector
temporary has an anonymous representative:
Partition 1 (_2 - 2 14 16 ) _2 has no base variable, while
14 and 16 are g18_lsm.11_14 and _16
Partition 30 (g18_lsm.11_41 - 41 )
Partition 1 is skipped because SSA_NAME_VAR of _2 is null, so partition 30
believes it is the only partition of g18_lsm.11 and keeps the user decl. Both
128 byte slots then get MEM_EXPR g18_lsm.11. The load/store pair-fusion pass
groups accesses by MEM_EXPR base and MEM_OFFSET, so it treated two stores that
are 144 bytes apart as adjacent and fused them:
before (set (mem:V4SI [sp+240]) (reg v17)) ; g18_lsm.11+96
(set (mem:V4SI [sp+384]) (reg v27)) ; g18_lsm.11+112
after (set (mem:V2x16QI [sp+240])
(unspec [(reg v17) (reg v27)] UNSPEC_STP))
[sp+384] is never written.
Work out the variable each partition will be given and key the keep/split
decision on that, reusing the function set_rtl makes the same choice with,
renamed to expand_leader_merge now.
That function keeps the variable it is given first unless
a later one is DECL_IGNORED_P, and expansion hands set_rtl the representative
before the other names, so the representative's variable goes in first.
A partition holding names of two variables would otherwise be keyed on the
wrong one.
Bootstrapped and tested on aarch64-none-linux-gnu.
gcc/ChangeLog:
PR middle-end/126405
* cfgexpand.cc (leader_merge): Rename to...
(expand_leader_merge): ...this. Make external.
* cfgexpand.h (expand_leader_merge): Declare.
* tree-outof-ssa.cc: Include cfgexpand.h.
(split_overlapping_partition_decls): Determine the variable of a
partition from all of its names.
gcc/testsuite/ChangeLog:
PR middle-end/126405
* gcc.c-torture/execute/pr126405.c: New test.
* gcc.c-torture/execute/pr126405-2.c: New test.
* gcc.dg/pr126405-1.c: New test.
* gcc.dg/pr126405-2.c: New test.
* gcc.dg/pr126405-3.c: New test.
Signed-off-by: Kyrylo Tkachov <[email protected]>
---
gcc/cfgexpand.cc | 10 ++--
gcc/cfgexpand.h | 1 +
.../gcc.c-torture/execute/pr126405-2.c | 54 ++++++++++++++++++
.../gcc.c-torture/execute/pr126405.c | 56 +++++++++++++++++++
gcc/testsuite/gcc.dg/pr126405-1.c | 29 ++++++++++
gcc/testsuite/gcc.dg/pr126405-2.c | 35 ++++++++++++
gcc/testsuite/gcc.dg/pr126405-3.c | 26 +++++++++
gcc/tree-outof-ssa.cc | 29 +++++++++-
8 files changed, 234 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr126405-2.c
create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr126405.c
create mode 100644 gcc/testsuite/gcc.dg/pr126405-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr126405-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr126405-3.c
diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 05efcea13ba..28b449c49f5 100644
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -155,10 +155,11 @@ gimple_assign_rhs_to_tree (gimple *stmt)
/* Choose either CUR or NEXT as the leader DECL for a partition.
Prefer ignored decls, to simplify debug dumps and reduce ambiguity
out of the same user variable being in multiple partitions (this is
- less likely for compiler-introduced temps). */
+ less likely for compiler-introduced temps). Also used by out-of-SSA
+ to work out which variable a partition will be given. */
-static tree
-leader_merge (tree cur, tree next)
+tree
+expand_leader_merge (tree cur, tree next)
{
if (cur == NULL || cur == next)
return next;
@@ -251,7 +252,8 @@ set_rtl (tree t, rtx x)
else
gcc_unreachable ();
- tree next = skip ? cur : leader_merge (cur, SSAVAR (t) ? SSAVAR (t) : t);
+ tree next
+ = skip ? cur : expand_leader_merge (cur, SSAVAR (t) ? SSAVAR (t) : t);
if (cur != next)
{
diff --git a/gcc/cfgexpand.h b/gcc/cfgexpand.h
index f3acb886225..feda4e44173 100644
--- a/gcc/cfgexpand.h
+++ b/gcc/cfgexpand.h
@@ -24,6 +24,7 @@ extern tree gimple_assign_rhs_to_tree (gimple *);
extern HOST_WIDE_INT estimated_stack_frame_size (struct cgraph_node *);
extern void expand_remove_edge (edge);
extern void set_parm_rtl (tree, rtx);
+extern tree expand_leader_merge (tree, tree);
#endif /* GCC_CFGEXPAND_H */
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr126405-2.c
b/gcc/testsuite/gcc.c-torture/execute/pr126405-2.c
new file mode 100644
index 00000000000..49ce58539a2
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr126405-2.c
@@ -0,0 +1,54 @@
+/* The same out-of-SSA defect as pr126405.c, with a narrower companion vector.
+ That changes the register pressure around the copy of the oversized vector
+ and so the pair the fusion pass picks, but the cause is the same: two stack
+ slots of one variable sharing a MEM_EXPR. Self-checking: aborts if the
+ result is wrong. */
+
+typedef long __attribute__((vector_size (16 * sizeof (long)))) v16di;
+typedef int __attribute__((vector_size (8 * sizeof (int)))) v8si;
+
+long g2, g12;
+v16di g18;
+v8si g3;
+void *g27;
+
+/* The wrong value is read from an uninitialised stack slot, so make sure the
+ stack the callee reuses does not happen to be zero. */
+__attribute__((noipa)) static void
+dirty_stack (void)
+{
+ volatile char buf[1024];
+ for (unsigned i = 0; i < sizeof (buf); i++)
+ buf[i] = 0xa5;
+}
+
+void
+f31 (void)
+{
+lbl_br1:
+ g18 = ~g18;
+ g3 = ~g3;
+ if (g2)
+ goto lbl_br1;
+lbl_b5:
+ switch (g12)
+ case 4:
+ case 0:
+ goto lbl_sw8;
+ __builtin_abort ();
+lbl_sw8:
+ if (g27)
+ goto lbl_b5;
+ g18 = ~g18;
+}
+
+int
+main (void)
+{
+ dirty_stack ();
+ f31 ();
+ for (int i = 0; i < 16; i++)
+ if (g18[i] != 0)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr126405.c
b/gcc/testsuite/gcc.c-torture/execute/pr126405.c
new file mode 100644
index 00000000000..84d54364e95
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr126405.c
@@ -0,0 +1,56 @@
+/* AArch64 wrong code at -O2. Store motion creates several SSA versions of an
+ oversized vector temporary (V16DI, 128 bytes, no register mode). The
+ partition holding the loop-carried versions has an anonymous representative,
+ so out-of-SSA left it and the partition of the copy taken for the use after
+ the loop sharing one MEM_EXPR. The load/store pair-fusion pass then treated
+ two stores 144 bytes apart as adjacent, fused them, and left the tail of one
+ slot uninitialised. Self-checking: aborts if the result is wrong. */
+
+typedef long __attribute__((vector_size (16 * sizeof (long)))) v16di;
+typedef int __attribute__((vector_size (16 * sizeof (int)))) v16si;
+
+long g2, g12;
+v16di g18;
+v16si g3;
+void *g27;
+
+/* The wrong value is read from an uninitialised stack slot, so make sure the
+ stack the callee reuses does not happen to be zero. */
+__attribute__((noipa)) static void
+dirty_stack (void)
+{
+ volatile char buf[1024];
+ for (unsigned i = 0; i < sizeof (buf); i++)
+ buf[i] = 0xa5;
+}
+
+void
+f31 (void)
+{
+lbl_br1:
+ g18 = ~g18;
+ g3 = ~g3;
+ if (g2)
+ goto lbl_br1;
+lbl_b5:
+ switch (g12)
+ case 4:
+ case 0:
+ goto lbl_sw8;
+ __builtin_abort ();
+lbl_sw8:
+ if (g27)
+ goto lbl_b5;
+ g18 = ~g18;
+}
+
+int
+main (void)
+{
+ dirty_stack ();
+ f31 ();
+ for (int i = 0; i < 16; i++)
+ if (g18[i] != 0)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr126405-1.c
b/gcc/testsuite/gcc.dg/pr126405-1.c
new file mode 100644
index 00000000000..40007b108eb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr126405-1.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fgimple -ffloat-store -ftree-coalesce-vars
-fdump-rtl-expand" } */
+
+/* The partition holding v_2 has an anonymous representative, _1, so the
+ variable that expansion attaches to its stack slot comes from a name other
+ than the representative. -ffloat-store puts both partitions of v in memory,
+ and they must not end up sharing a MEM_EXPR. */
+
+double in1, in2, out1, out2, out3, out4;
+
+void __GIMPLE (ssa, startwith ("expand"))
+f (void)
+{
+ double v;
+ double _1;
+
+__BB(2):
+ _1 = in1;
+ v_2 = _1;
+ v_3 = in2;
+ out1 = v_2;
+ out2 = v_3;
+ out3 = v_2;
+ out4 = v_3;
+ return;
+}
+
+/* { dg-final { scan-rtl-dump {\[[0-9]+ v\+0} "expand" } } */
+/* { dg-final { scan-rtl-dump {\[[0-9]+ D\.[0-9]+\+0} "expand" } } */
diff --git a/gcc/testsuite/gcc.dg/pr126405-2.c
b/gcc/testsuite/gcc.dg/pr126405-2.c
new file mode 100644
index 00000000000..c320824e4e4
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr126405-2.c
@@ -0,0 +1,35 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fgimple -ffloat-store -ftree-coalesce-vars
-fdump-rtl-expand" } */
+
+/* Three partitions of v, each with an anonymous representative. One keeps v
+ and the other two need their own decls, so that no two of the three stack
+ slots share a MEM_EXPR. */
+
+double in1, in2, in3, out1, out2, out3, out4, out5, out6;
+
+void __GIMPLE (ssa, startwith ("expand"))
+f (void)
+{
+ double v;
+ double _1;
+ double _3;
+ double _5;
+
+__BB(2):
+ _1 = in1;
+ v_2 = _1;
+ _3 = in2;
+ v_4 = _3;
+ _5 = in3;
+ v_6 = _5;
+ out1 = v_2;
+ out2 = v_4;
+ out3 = v_6;
+ out4 = v_2;
+ out5 = v_4;
+ out6 = v_6;
+ return;
+}
+
+/* { dg-final { scan-rtl-dump {\[[0-9]+ v\+0} "expand" } } */
+/* { dg-final { scan-rtl-dump {\[[0-9]+ D\.[0-9]+\+0} "expand" } } */
diff --git a/gcc/testsuite/gcc.dg/pr126405-3.c
b/gcc/testsuite/gcc.dg/pr126405-3.c
new file mode 100644
index 00000000000..b6e1bdc2498
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr126405-3.c
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -ffloat-store -ftree-coalesce-vars -fdump-rtl-expand" } */
+
+/* Coalescing puts names of u and of v in one partition, and its representative
+ is a name of v rather than the lowest numbered member. The variable that
+ expansion attaches to that partition is therefore the one of the
+ representative, and keying the split on any other member of the partition
+ leaves it and the second partition of v sharing a MEM_EXPR. */
+
+double g1, g2, g3, g4, g5;
+
+void
+f (int n)
+{
+ double u = g1;
+ double v = u;
+ for (int i = 0; i < n; i++)
+ v = v + g3;
+ double t = v;
+ v = g2;
+ g4 = t;
+ g5 = v;
+}
+
+/* { dg-final { scan-rtl-dump {\[[0-9]+ v\+0} "expand" } } */
+/* { dg-final { scan-rtl-dump {\[[0-9]+ D\.[0-9]+\+0} "expand" } } */
diff --git a/gcc/tree-outof-ssa.cc b/gcc/tree-outof-ssa.cc
index 17d5e70e71d..309acd4fde2 100644
--- a/gcc/tree-outof-ssa.cc
+++ b/gcc/tree-outof-ssa.cc
@@ -45,6 +45,7 @@ along with GCC; see the file COPYING3. If not see
#include "tree-ssa-ter.h"
#include "tree-ssa-coalesce.h"
#include "tree-outof-ssa.h"
+#include "cfgexpand.h"
#include "dojump.h"
#include "internal-fn.h"
#include "gimple-fold.h"
@@ -1072,13 +1073,39 @@ split_overlapping_partition_decls (var_map map)
auto_vec<tree> new_decl;
new_decl.safe_grow_cleared (n);
bool any = false;
+ unsigned ver;
+ tree name;
+
+ /* set_rtl attaches the base variable of any name in a partition to that
+ partition's location, not just the one of its representative, so collect
+ what the names of each partition contribute. A name with no base
+ variable contributes nothing, since set_rtl passes a type rather than a
+ decl for those and leaves the MEM_EXPR it has in place. */
+ auto_vec<tree> part_var;
+ part_var.safe_grow_cleared (n);
+ FOR_EACH_SSA_NAME (ver, name, cfun)
+ {
+ int p = var_to_partition (map, name);
+ if (p == NO_PARTITION)
+ continue;
+ tree var = SSA_NAME_VAR (name);
+ if (!var)
+ continue;
+ part_var[p] = expand_leader_merge (part_var[p], var);
+ }
for (unsigned i = 0; i < n; i++)
{
tree repr = partition_to_var (map, i);
if (!repr)
continue;
+ /* 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. */
tree var = SSA_NAME_VAR (repr);
+ if (part_var[i])
+ var = expand_leader_merge (var, part_var[i]);
if (!var || !VAR_P (var))
continue;
/* Only partitions that will live in memory can end up with a
@@ -1123,8 +1150,6 @@ split_overlapping_partition_decls (var_map map)
if (!any)
return;
- unsigned ver;
- tree name;
FOR_EACH_SSA_NAME (ver, name, cfun)
{
if (SSA_NAME_IS_DEFAULT_DEF (name))
--
2.50.1 (Apple Git-155)
