On Tue, 30 Oct 2018, Richard Biener wrote:
>
> This picks up work from earlier this year where Aldy worked on
> undoing forwprop during out-of-SSA to improve coalescing across
> backedges.
>
> The following patch first rectifies the existing code which
> is meant to insert necessary copies in places where it then
> allows coalescing and thus avoids splitting of the backedge.
>
> The current code has issues with handling conflicts with uses
> in the exit condition badly which is why the patch instead
> of on the backedge inserts the copy before the definition
> of the backedge value. It also expands the constraint of
> handling only single-BB loops (because of trivially_conflicts_p
> restrictions). Also we can coalesce vars with different
> SSA_NAME_VAR just fine now.
>
> This makes the cases in the PRs keep their natural loop
> form where originally they had their backedge split because
> the inserted copies didn't do the job.
>
> The testcases may be a bit awkward (and hopefully survive
> solaris assembler woes...)
>
> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
The following is what I have applied.
Richard.
>From 5b774992b3863931ccbba13effc9efcea304610e Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguent...@suse.de>
Date: Tue, 30 Oct 2018 14:46:05 +0100
Subject: [PATCH] fix-pr70359
PR middle-end/70359
PR middle-end/86270
* tree-outof-ssa.c (insert_backedge_copies): Restrict
copy generation to useful cases. Place the copy before
the definition of the backedge value when possible.
* gcc.target/i386/pr70359.c: New testcase.
* gcc.target/i386/pr86270.c: Likewise.
diff --git a/gcc/testsuite/gcc.target/i386/pr70359.c
b/gcc/testsuite/gcc.target/i386/pr70359.c
new file mode 100644
index 00000000000..85b7017e386
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr70359.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+char* inttostr(int i, char* buf, int len)
+{
+ unsigned int ui = (i > 0) ? i : -i;
+ char *p = buf + len - 1;
+ *p = '\0';
+ do {
+ *--p = '0' + (ui % 10);
+ } while ((ui /= 10) != 0);
+ if (i < 0) {
+ *--p = '-';
+ }
+ return p;
+}
+
+/* In out-of-SSA we should have avoided splitting the latch edge of the
+ loop by inserting copies. */
+/* { dg-final { scan-assembler-times "L\[0-9\]+:" 2 } } */
diff --git a/gcc/testsuite/gcc.target/i386/pr86270.c
b/gcc/testsuite/gcc.target/i386/pr86270.c
new file mode 100644
index 00000000000..81841ef5bd7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr86270.c
@@ -0,0 +1,15 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+int *a;
+long len;
+
+int
+test ()
+{
+ for (int i = 0; i < len + 1; i++)
+ a[i]=i;
+}
+
+/* Check we do not split the backedge but keep nice loop form. */
+/* { dg-final { scan-assembler-times "L\[0-9\]+:" 2 } } */
diff --git a/gcc/tree-outof-ssa.c b/gcc/tree-outof-ssa.c
index fca15e5f898..fd442d5e0a0 100644
--- a/gcc/tree-outof-ssa.c
+++ b/gcc/tree-outof-ssa.c
@@ -1171,15 +1171,19 @@ insert_backedge_copies (void)
{
tree arg = gimple_phi_arg_def (phi, i);
edge e = gimple_phi_arg_edge (phi, i);
+ /* We are only interested in copies emitted on critical
+ backedges. */
+ if (!(e->flags & EDGE_DFS_BACK)
+ || !EDGE_CRITICAL_P (e))
+ continue;
/* If the argument is not an SSA_NAME, then we will need a
- constant initialization. If the argument is an SSA_NAME with
- a different underlying variable then a copy statement will be
- needed. */
- if ((e->flags & EDGE_DFS_BACK)
- && (TREE_CODE (arg) != SSA_NAME
- || SSA_NAME_VAR (arg) != SSA_NAME_VAR (result)
- || trivially_conflicts_p (bb, result, arg)))
+ constant initialization. If the argument is an SSA_NAME then
+ a copy statement may be needed. First handle the case
+ where we cannot insert before the argument definition. */
+ if (TREE_CODE (arg) != SSA_NAME
+ || (gimple_code (SSA_NAME_DEF_STMT (arg)) == GIMPLE_PHI
+ && trivially_conflicts_p (bb, result, arg)))
{
tree name;
gassign *stmt;
@@ -1226,6 +1230,34 @@ insert_backedge_copies (void)
gsi_insert_after (&gsi2, stmt, GSI_NEW_STMT);
SET_PHI_ARG_DEF (phi, i, name);
}
+ /* Insert a copy before the definition of the backedge value
+ and adjust all conflicting uses. */
+ else if (trivially_conflicts_p (bb, result, arg))
+ {
+ gimple *def = SSA_NAME_DEF_STMT (arg);
+ if (gimple_nop_p (def)
+ || gimple_code (def) == GIMPLE_PHI)
+ continue;
+ tree name = copy_ssa_name (result);
+ gimple *stmt = gimple_build_assign (name, result);
+ imm_use_iterator imm_iter;
+ gimple *use_stmt;
+ /* The following matches trivially_conflicts_p. */
+ FOR_EACH_IMM_USE_STMT (use_stmt, imm_iter, result)
+ {
+ if (gimple_bb (use_stmt) != bb
+ || (gimple_code (use_stmt) != GIMPLE_PHI
+ && (maybe_renumber_stmts_bb (bb), true)
+ && gimple_uid (use_stmt) > gimple_uid (def)))
+ {
+ use_operand_p use;
+ FOR_EACH_IMM_USE_ON_STMT (use, imm_iter)
+ SET_USE (use, name);
+ }
+ }
+ gimple_stmt_iterator gsi = gsi_for_stmt (def);
+ gsi_insert_before (&gsi, stmt, GSI_SAME_STMT);
+ }
}
}
