* [PATCH] Fix PRs70359/86270
@ 2018-10-30 14:50 Richard Biener
2018-10-31 13:53 ` Richard Biener
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2018-10-30 14:50 UTC (permalink / raw)
To: gcc-patches
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.
Richard.
From 812187f4eabef40a42594bad48244047f3b37b89 Mon Sep 17 00:00:00 2001
From: Richard Guenther <rguenther@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..efb75c207e6 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);
+ if (!(e->flags & EDGE_DFS_BACK)
+ /* If the backedge is already split there's nothing
+ to avoid. */
+ || e->src != bb)
+ 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);
+ }
}
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PRs70359/86270
2018-10-30 14:50 [PATCH] Fix PRs70359/86270 Richard Biener
@ 2018-10-31 13:53 ` Richard Biener
2018-11-01 14:37 ` H.J. Lu
0 siblings, 1 reply; 3+ messages in thread
From: Richard Biener @ 2018-10-31 13:53 UTC (permalink / raw)
To: gcc-patches
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 <rguenther@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);
+ }
}
}
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix PRs70359/86270
2018-10-31 13:53 ` Richard Biener
@ 2018-11-01 14:37 ` H.J. Lu
0 siblings, 0 replies; 3+ messages in thread
From: H.J. Lu @ 2018-11-01 14:37 UTC (permalink / raw)
To: Richard Guenther; +Cc: GCC Patches
On Wed, Oct 31, 2018 at 4:56 AM Richard Biener <rguenther@suse.de> wrote:
>
> 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 <rguenther@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.
>
This caused:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87852
--
H.J.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-11-01 14:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-30 14:50 [PATCH] Fix PRs70359/86270 Richard Biener
2018-10-31 13:53 ` Richard Biener
2018-11-01 14:37 ` H.J. Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).