diff mbox

Fix endless loop in forwprop (PR tree-optimization/53226)

Message ID 20120509122301.GR16117@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek May 9, 2012, 12:23 p.m. UTC
On Wed, May 09, 2012 at 10:21:04AM +0200, Richard Guenther wrote:
> Using the stmt UID looks odd as you are using it as flag - why not use
> one of the two stmt flags available to passes?
> 
> Btw, the other possibility would be to simply change the various combiners
> to require them to update the iterator to point at the first statement that
> they have inserted.  I have some TLC ideas in this area, let's see if I can
> get to them.
> 
> Ok with using gimple_set_plf and friends.

Here is what I've committed after doing another bootstrap/regtest:

2012-05-09  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/53226
	* tree-ssa-forwprop.c (ssa_forward_propagate_and_combine): Remove
	prev and prev_initialized vars, gimple_set_plf (stmt, GF_PLF_1, false)
	before processing it and gimple_set_plf (stmt, GF_PLF_1, true) if it
	doesn't need to be revisited, look for earliest stmt with
	!gimple_plf (stmt, GF_PLF_1) if something changed.

	* gcc.c-torture/compile/pr53226.c: New test.


	Jakub
diff mbox

Patch

--- gcc/tree-ssa-forwprop.c.jj	2012-05-03 08:35:52.000000000 +0200
+++ gcc/tree-ssa-forwprop.c	2012-05-08 18:10:19.662061709 +0200
@@ -2677,8 +2677,7 @@  ssa_forward_propagate_and_combine (void)
 
   FOR_EACH_BB (bb)
     {
-      gimple_stmt_iterator gsi, prev;
-      bool prev_initialized;
+      gimple_stmt_iterator gsi;
 
       /* Apply forward propagation to all stmts in the basic-block.
 	 Note we update GSI within the loop as necessary.  */
@@ -2771,12 +2770,14 @@  ssa_forward_propagate_and_combine (void)
 
       /* Combine stmts with the stmts defining their operands.
 	 Note we update GSI within the loop as necessary.  */
-      prev_initialized = false;
       for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);)
 	{
 	  gimple stmt = gsi_stmt (gsi);
 	  bool changed = false;
 
+	  /* Mark stmt as potentially needing revisiting.  */
+	  gimple_set_plf (stmt, GF_PLF_1, false);
+
 	  switch (gimple_code (stmt))
 	    {
 	    case GIMPLE_ASSIGN:
@@ -2856,18 +2857,18 @@  ssa_forward_propagate_and_combine (void)
 	    {
 	      /* If the stmt changed then re-visit it and the statements
 		 inserted before it.  */
-	      if (!prev_initialized)
+	      for (; !gsi_end_p (gsi); gsi_prev (&gsi))
+		if (gimple_plf (gsi_stmt (gsi), GF_PLF_1))
+		  break;
+	      if (gsi_end_p (gsi))
 		gsi = gsi_start_bb (bb);
 	      else
-		{
-		  gsi = prev;
-		  gsi_next (&gsi);
-		}
+		gsi_next (&gsi);
 	    }
 	  else
 	    {
-	      prev = gsi;
-	      prev_initialized = true;
+	      /* Stmt no longer needs to be revisited.  */
+	      gimple_set_plf (stmt, GF_PLF_1, true);
 	      gsi_next (&gsi);
 	    }
 	}
--- gcc/testsuite/gcc.c-torture/compile/pr53226.c.jj	2012-05-08 18:07:40.007000510 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr53226.c	2012-05-08 18:07:35.071029578 +0200
@@ -0,0 +1,13 @@ 
+/* PR tree-optimization/53226 */
+
+void
+foo (unsigned long *x, char y, char z)
+{
+  int i;
+  for (i = y; i < z; ++i)
+    {
+      unsigned long a = ((unsigned char) i) & 63UL;
+      unsigned long b = 1ULL << a;
+      *x |= b;
+    }
+}