Patchwork Check headers in verify_loop_structure

login
register
mail settings
Submitter Marek Polacek
Date Feb. 7, 2013, 4:42 p.m.
Message ID <20130207164229.GK7360@redhat.com>
Download mbox | patch
Permalink /patch/218952/
State New
Headers show

Comments

Marek Polacek - Feb. 7, 2013, 4:42 p.m.
On Thu, Feb 07, 2013 at 02:56:48PM +0100, Richard Biener wrote:
> > +  /* 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

Done.

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

Fixed.  Regtested/bootstrapped on x86_64 again (with tailc fix:
http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00310.html), ok now?

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

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


	Marek
Richard Guenther - Feb. 8, 2013, 11:20 a.m.
On Thu, 7 Feb 2013, Marek Polacek wrote:

> On Thu, Feb 07, 2013 at 02:56:48PM +0100, Richard Biener wrote:
> > > +  /* 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
> 
> Done.
> 
> > > +      if (bb_loop_header_p (bb))
> > > +	if (bb->loop_father->header != bb)
> > 
> >   && bb->loop_father->header != bb)
> 
> Fixed.  Regtested/bootstrapped on x86_64 again (with tailc fix:
> http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00310.html), ok now?

Ok.  Let's watch for fallout...

Thanks,
Richard.

> 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 15:06:19.278571095 +0100
> @@ -1353,6 +1353,15 @@ verify_loop_structure (void)
>  	}
>      }
>  
> +  /* Check the headers.  */
> +  FOR_EACH_BB (bb)
> +    if (bb_loop_header_p (bb)
> +	&& 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);
> 
> 	Marek
> 
>
Toon Moene - Feb. 8, 2013, 11:38 a.m.
On 02/08/2013 12:20 PM, Richard Biener wrote:

> On Thu, 7 Feb 2013, Marek Polacek wrote:
>
>> On Thu, Feb 07, 2013 at 02:56:48PM +0100, Richard Biener wrote:
>>>> +  /* 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
>>
>> Done.
>>
>>>> +      if (bb_loop_header_p (bb))
>>>> +	if (bb->loop_father->header != bb)
>>>
>>>    &&  bb->loop_father->header != bb)
>>
>> Fixed.  Regtested/bootstrapped on x86_64 again (with tailc fix:
>> http://gcc.gnu.org/ml/gcc-patches/2013-02/msg00310.html), ok now?
>
> Ok.  Let's watch for fallout...
>
> Thanks,
> Richard.

Maybe here:

http://gcc.gnu.org/ml/gcc-testresults/2013-02/msg00835.html

?
Marek Polacek - Feb. 8, 2013, 11:40 a.m.
On Fri, Feb 08, 2013 at 12:20:18PM +0100, Richard Biener wrote:
> Ok.  Let's watch for fallout...

Yeah, we'll see.  Commited.

	Marek
Marek Polacek - Feb. 8, 2013, 11:42 a.m.
On Fri, Feb 08, 2013 at 12:38:48PM +0100, Toon Moene wrote:
> Maybe here:
> 
> http://gcc.gnu.org/ml/gcc-testresults/2013-02/msg00835.html

Well this one is definitely not caused by the new verifier bits above.

	Marek
Richard Guenther - Feb. 8, 2013, 11:45 a.m.
On Fri, 8 Feb 2013, Marek Polacek wrote:

> On Fri, Feb 08, 2013 at 12:38:48PM +0100, Toon Moene wrote:
> > Maybe here:
> > 
> > http://gcc.gnu.org/ml/gcc-testresults/2013-02/msg00835.html
> 
> Well this one is definitely not caused by the new verifier bits above.

I've seen this locally when testing a LTO patch and am testing a fix
right now (it's the IRA changes).

Richard.

Patch

--- gcc/cfgloop.c.mp	2013-02-07 14:07:24.580864372 +0100
+++ gcc/cfgloop.c	2013-02-07 15:06:19.278571095 +0100
@@ -1353,6 +1353,15 @@  verify_loop_structure (void)
 	}
     }
 
+  /* Check the headers.  */
+  FOR_EACH_BB (bb)
+    if (bb_loop_header_p (bb)
+	&& 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);