diff mbox

Fix PR55833 + cheaper checking

Message ID 20130110173143.GD20218@redhat.com
State New
Headers show

Commit Message

Marek Polacek Jan. 10, 2013, 5:31 p.m. UTC 
The following patch fixes (?) PR55833 by recomputing irreducible loops
after every unswitch-transformation.  With this testcase, when we perform
loop unswitching on the RTL level, we end up in situation where we have
a loop into which we get through loop exit of another loop which is
a part of an irreducible region.  Between those two loops is something like
preheader, which isn't marked as irreducible, nor its edges.  verify_loop_structure
then ICEs because of that.  Another hunk of this patch is just cheaper checking,
written by Richi.

Regtested/bootstrapped on x86_64-linux.

Zdenek, any thoughts on this?

2013-01-10  Richard Biener  <rguenther@suse.de>
	    Marek Polacek  <polacek@redhat.com>

	PR rtl-optimization/55833
	* loop-unswitch.c (unswitch_loops): Move loop verification...
	(unswitch_single_loop): ...here.  Call mark_irreducible_loops.

	* gcc.dg/pr55833.c: New test.


	Marek

Comments

Steven Bosscher Jan. 10, 2013, 5:41 p.m. UTC | #1 
On Thu, Jan 10, 2013 at 6:31 PM, Marek Polacek wrote:
> +  /* We changed the CFG.  Recompute irreducible BBs and edges.  */
> +  mark_irreducible_loops ();

This is a very expensive fix for a really unusual situation.

I don't think this is the right thing to do...

Ciao!
Steven
Zdenek Dvorak Jan. 10, 2013, 10:19 p.m. UTC | #2 
Hi,

> On Thu, Jan 10, 2013 at 6:31 PM, Marek Polacek wrote:
> > +  /* We changed the CFG.  Recompute irreducible BBs and edges.  */
> > +  mark_irreducible_loops ();
> 
> This is a very expensive fix for a really unusual situation.
> 
> I don't think this is the right thing to do...

I agree -- at the very least, unswitch_single_loop should check whether there
is any possiblity it could have affected irreducible loops information (this
should only be the case when some already existing irreducible loop is altered
in the progress).  Which is what it (or more precisely, remove_path function
used by it) tries to do -- so is should be sufficient to check why this fails
for the considered testcase, and make sure the situation is correctly detected,

Zdenek
diff mbox

Patch

--- gcc/loop-unswitch.c.mp	2013-01-10 16:50:28.899559875 +0100
+++ gcc/loop-unswitch.c	2013-01-10 16:50:34.203575403 +0100
@@ -145,12 +145,7 @@  unswitch_loops (void)
   /* Go through inner loops (only original ones).  */
 
   FOR_EACH_LOOP (li, loop, LI_ONLY_INNERMOST)
-    {
-      unswitch_single_loop (loop, NULL_RTX, 0);
-#ifdef ENABLE_CHECKING
-      verify_loop_structure ();
-#endif
-    }
+    unswitch_single_loop (loop, NULL_RTX, 0);
 
   iv_analysis_done ();
 }
@@ -370,6 +365,13 @@  unswitch_single_loop (struct loop *loop,
   nloop = unswitch_loop (loop, bbs[i], copy_rtx_if_shared (cond), cinsn);
   gcc_assert (nloop);
 
+  /* We changed the CFG.  Recompute irreducible BBs and edges.  */
+  mark_irreducible_loops ();
+
+#ifdef ENABLE_CHECKING
+  verify_loop_structure ();
+#endif
+
   /* Invoke itself on modified loops.  */
   unswitch_single_loop (nloop, rconds, num + 1);
   unswitch_single_loop (loop, conds, num + 1);
--- gcc/testsuite/gcc.dg/pr55833.c.mp	2013-01-10 17:23:26.016102692 +0100
+++ gcc/testsuite/gcc.dg/pr55833.c	2013-01-10 17:23:15.898073384 +0100
@@ -0,0 +1,28 @@ 
+/* PR rtl-optimization/55833 */
+/* { dg-do compile } */
+/* { dg-options "-O3" } */
+
+int a, b, c;
+
+void foo()
+{
+    unsigned d, l, *p, k = 1;
+
+    if(bar())
+    {
+label:
+      	if((a = a <= 0))
+        {
+            if(c)
+                d = b;
+
+            if (b || d ? l : k ? : 0)
+                a = d = 0;
+
+            goto label;
+       	}
+    }
+
+    while(*p++)
+        goto label;
+}