Patchwork Check headers in verify_loop_structure

login
register
mail settings
Submitter Marek Polacek
Date Feb. 7, 2013, 1:46 p.m.
Message ID <20130207134638.GI7360@redhat.com>
Download mbox | patch
Permalink /patch/218930/
State New
Headers show

Comments

Marek Polacek - Feb. 7, 2013, 1:46 p.m.
On Thu, Feb 07, 2013 at 10:14:14AM +0100, Richard Biener wrote:
> I'd say "loop with header %d not in loop tree" here.  Eventually
> the header detection should be shared with the code in flow_loops_find
> to avoid divergence.  I'm making changes to the flow_loops_find code
> at the moment and will look at splitting out that part properly to
> be re-used by the verifier.

Ok, thanks.  This version, on top of your PR56181 fix, uses
bb_loop_header_p then.  Regtested/bootstrapped on x86_64-linux,
ok for trunk after you commit your patch?

Maybe I should try bootstrap-O3 on it as well...

2013-02-07  Marek Polacek  <polacek@redhat.com>

	* cfgloop.c (verify_loop_structure): Add more checking
	of headers.


	Marek
Richard Guenther - Feb. 7, 2013, 1:56 p.m.
On Thu, 7 Feb 2013, Marek Polacek wrote:

> On Thu, Feb 07, 2013 at 10:14:14AM +0100, Richard Biener wrote:
> > I'd say "loop with header %d not in loop tree" here.  Eventually
> > the header detection should be shared with the code in flow_loops_find
> > to avoid divergence.  I'm making changes to the flow_loops_find code
> > at the moment and will look at splitting out that part properly to
> > be re-used by the verifier.
> 
> Ok, thanks.  This version, on top of your PR56181 fix, uses
> bb_loop_header_p then.  Regtested/bootstrapped on x86_64-linux,
> ok for trunk after you commit your patch?
> 
> Maybe I should try bootstrap-O3 on it as well...
> 
> 2013-02-07  Marek Polacek  <polacek@redhat.com>
> 
> 	* cfgloop.c (verify_loop_structure): Add more checking
> 	of headers.
> 
> --- gcc/cfgloop.c.mp	2013-02-07 14:07:24.580864372 +0100
> +++ gcc/cfgloop.c	2013-02-07 14:11:42.005604064 +0100
> @@ -1353,6 +1353,21 @@ verify_loop_structure (void)
>  	}
>      }
>  
> +  /* Check the headers.  */
> +  FOR_EACH_BB (bb)
> +    {
> +      /* Skip BBs in the root tree.  */
> +      if (bb->loop_father == current_loops->tree_root)
> +	continue;

You shouldn't need this ... it will miss missing toplevel loops

> +      if (bb_loop_header_p (bb))
> +	if (bb->loop_father->header != bb)

  && bb->loop_father->header != bb)

Richard.

> +	  {
> +	    error ("loop with header %d not in loop tree", bb->index);
> +	    err = 1;
> +	  }
> +    }
> +
>    /* Check get_loop_body.  */
>    visited = sbitmap_alloc (last_basic_block);
>    bitmap_clear (visited);

Patch

--- gcc/cfgloop.c.mp	2013-02-07 14:07:24.580864372 +0100
+++ gcc/cfgloop.c	2013-02-07 14:11:42.005604064 +0100
@@ -1353,6 +1353,21 @@  verify_loop_structure (void)
 	}
     }
 
+  /* Check the headers.  */
+  FOR_EACH_BB (bb)
+    {
+      /* Skip BBs in the root tree.  */
+      if (bb->loop_father == current_loops->tree_root)
+	continue;
+
+      if (bb_loop_header_p (bb))
+	if (bb->loop_father->header != bb)
+	  {
+	    error ("loop with header %d not in loop tree", bb->index);
+	    err = 1;
+	  }
+    }
+
   /* Check get_loop_body.  */
   visited = sbitmap_alloc (last_basic_block);
   bitmap_clear (visited);