[PATCH] Fix PR77514

[PATCH] Fix PR77514

[Richard Biener]

Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.

Richard.

2016-09-15  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/77514
	* tree-ssa-pre.c (create_expression_by_pieces): Handle garbage
	only forced_stmts sequence.

	* gcc.dg/torture/pr77514.c: New testcase.

Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c	(revision 240133)
+++ gcc/tree-ssa-pre.c	(working copy)
@@ -2879,7 +2879,21 @@ create_expression_by_pieces (basic_block
       gimple_seq_discard (forced_stmts);
       return folded;
     }
-
+  /* Likewise if we simplified to sth not queued for insertion.  */
+  bool found = false;
+  gsi = gsi_start (forced_stmts);
+  for (; !gsi_end_p (gsi); gsi_next (&gsi))
+    {
+      gimple *stmt = gsi_stmt (gsi);
+      tree forcedname = gimple_get_lhs (stmt);
+      if (forcedname == folded)
+	found = true;
+    }
+  if (! found)
+    {
+      gimple_seq_discard (forced_stmts);
+      return folded;
+    }
   gcc_assert (TREE_CODE (folded) == SSA_NAME);
 
   /* If we have any intermediate expressions to the value sets, add them
Index: gcc/testsuite/gcc.dg/torture/pr77514.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr77514.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr77514.c	(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+
+void
+m1 (char l0, char e8, int hw)
+{
+  char *rs = &l0;
+
+yu:
+  l0 = 1;
+  while (l0 != 0)
+    {
+      l0 = -l0;
+      l0 += (*rs ^ (l0 &= 1));
+    }
+  for (;;)
+    {
+      if (hw != 0)
+	goto yu;
+      rs = &e8;
+    }
+}

Re: [PATCH] Fix PR77514

[Bernhard Reutner-Fischer]

On 15 September 2016 09:08:36 CEST, Richard Biener <rguenther@suse.de> wrote:
>
>Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
>

>+  /* Likewise if we simplified to sth not queued for insertion.  */
>+  bool found = false;
>+  gsi = gsi_start (forced_stmts);
>+  for (; !gsi_end_p (gsi); gsi_next (&gsi))
>+    {
>+      gimple *stmt = gsi_stmt (gsi);
>+      tree forcedname = gimple_get_lhs (stmt);
>+      if (forcedname == folded)
{
>+	found = true;

break;}

Or do we fix up such code anyway somewhere?
thanks,
>+    }
>+  if (! found)

Re: [PATCH] Fix PR77514

[Richard Biener]

On Fri, 16 Sep 2016, Bernhard Reutner-Fischer wrote:

> On 15 September 2016 09:08:36 CEST, Richard Biener <rguenther@suse.de> wrote:
> >
> >Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> >
> 
> >+  /* Likewise if we simplified to sth not queued for insertion.  */
> >+  bool found = false;
> >+  gsi = gsi_start (forced_stmts);
> >+  for (; !gsi_end_p (gsi); gsi_next (&gsi))
> >+    {
> >+      gimple *stmt = gsi_stmt (gsi);
> >+      tree forcedname = gimple_get_lhs (stmt);
> >+      if (forcedname == folded)
> {
> >+	found = true;
> 
> break;}
> 
> Or do we fix up such code anyway somewhere?

We don't -- I guess it would be an interesting transform but difficult
in this case as gimple_get_lhs might have side-effects we cannot skip.

It might fit loop splitting which would split it "dynamically" into
a head running until we set found to true, skipping to the tail
that has the loop with omitted guard and found = true setting.
If the loop ends up with no side-effects it should end up being
removed.  If not it might actually not be a desirable transform due
to code-size.

I'll fixup the PRE code.

Richard.

Re: [PATCH] Fix PR77514

[Michael Matz]

Hi,

On Fri, 16 Sep 2016, Richard Biener wrote:

> We don't -- I guess it would be an interesting transform but difficult 
> in this case as gimple_get_lhs might have side-effects we cannot skip.

If there are side-effects it's anyway not going to be very profitable.  
loop-splitting is mostly an enabler for vectorization; loops with 
side-effects don't vectorize (as of now).

> It might fit loop splitting which would split it "dynamically" into a 
> head running until we set found to true, skipping to the tail that has 
> the loop with omitted guard and found = true setting. If the loop ends 
> up with no side-effects it should end up being removed.  If not it might 
> actually not be a desirable transform due to code-size.

Yes, such transformation would be possible.  If the loop has no 
other side-effects than setting LCSSA names to a single value then the 
second loop wouldn't even be necessary (i.e. the "break" would essentially 
be inserted).  For this specific case it indeed looks like the loop 
splitting routines could be reasonably be used.


Ciao,
Michael.

Re: [PATCH] Fix PR77514

[Richard Biener]

On Fri, 16 Sep 2016, Richard Biener wrote:

> On Fri, 16 Sep 2016, Bernhard Reutner-Fischer wrote:
> 
> > On 15 September 2016 09:08:36 CEST, Richard Biener <rguenther@suse.de> wrote:
> > >
> > >Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk.
> > >
> > 
> > >+  /* Likewise if we simplified to sth not queued for insertion.  */
> > >+  bool found = false;
> > >+  gsi = gsi_start (forced_stmts);
> > >+  for (; !gsi_end_p (gsi); gsi_next (&gsi))
> > >+    {
> > >+      gimple *stmt = gsi_stmt (gsi);
> > >+      tree forcedname = gimple_get_lhs (stmt);
> > >+      if (forcedname == folded)
> > {
> > >+	found = true;
> > 
> > break;}
> > 
> > Or do we fix up such code anyway somewhere?
> 
> We don't -- I guess it would be an interesting transform but difficult
> in this case as gimple_get_lhs might have side-effects we cannot skip.
> 
> It might fit loop splitting which would split it "dynamically" into
> a head running until we set found to true, skipping to the tail
> that has the loop with omitted guard and found = true setting.
> If the loop ends up with no side-effects it should end up being
> removed.  If not it might actually not be a desirable transform due
> to code-size.
> 
> I'll fixup the PRE code.

Bootstrapped / tested on x86_64-unknown-linux-gnu.

Richard.

2016-09-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/77514
	* tree-ssa-pre.c (create_expression_by_pieces): Optimize
	search for folded stmt.

Index: gcc/tree-ssa-pre.c
===================================================================
--- gcc/tree-ssa-pre.c	(revision 240176)
+++ gcc/tree-ssa-pre.c	(working copy)
@@ -2881,13 +2881,16 @@ create_expression_by_pieces (basic_block
     }
   /* Likewise if we simplified to sth not queued for insertion.  */
   bool found = false;
-  gsi = gsi_start (forced_stmts);
-  for (; !gsi_end_p (gsi); gsi_next (&gsi))
+  gsi = gsi_last (forced_stmts);
+  for (; !gsi_end_p (gsi); gsi_prev (&gsi))
     {
       gimple *stmt = gsi_stmt (gsi);
       tree forcedname = gimple_get_lhs (stmt);
       if (forcedname == folded)
-	found = true;
+	{
+	  found = true;
+	  break;
+	}
     }
   if (! found)
     {