Fix PR56035

On Tue, Jan 22, 2013 at 06:44:40PM +0100, Zdenek Dvorak wrote:
> > I was however worried that we might end up with an edge
> > coming out of BB
> > from different loop heading into the header.  In this case setting up
> > loop's latch as the source of the latch edge would be wrong.
> 
> Actually, the comment suggesting that possibility does not make much sense.
> A latch (a predecessor of the header H that is dominated by H) belongs to
> the loop headed by H by definition, so I am not quite sure what the test was supposed to do.
> 
> The latch block may of course belong to a subloop of the loop, but that is not
> forbidden (and it is taken care of further in fix_loop_structure through force_single_succ_latches
> in the situations where we want to avoid this possibility),

Well, okay then, thanks.  So below is the new version of the patch.
Regtested/bootstrapped on x86_64-linux again, ok for trunk?

Richi, are you with this one?

2013-01-22  Marek Polacek  <polacek@redhat.com>

	* cfgloopmanip.c (fix_loop_structure): Remove redundant condition.

	* testsuite/gcc.dg/pr56035.c: New test.


	Marek

Marek Polacek <polacek@redhat.com> wrote:

>On Tue, Jan 22, 2013 at 06:44:40PM +0100, Zdenek Dvorak wrote:
>> > I was however worried that we might end up with an edge
>> > coming out of BB
>> > from different loop heading into the header.  In this case setting
>up
>> > loop's latch as the source of the latch edge would be wrong.
>> 
>> Actually, the comment suggesting that possibility does not make much
>sense.
>> A latch (a predecessor of the header H that is dominated by H)
>belongs to
>> the loop headed by H by definition, so I am not quite sure what the
>test was supposed to do.
>> 
>> The latch block may of course belong to a subloop of the loop, but
>that is not
>> forbidden (and it is taken care of further in fix_loop_structure
>through force_single_succ_latches
>> in the situations where we want to avoid this possibility),
>
>Well, okay then, thanks.  So below is the new version of the patch.
>Regtested/bootstrapped on x86_64-linux again, ok for trunk?
>
>Richi, are you with this one?

Yes. I cannot quite remember why I added this extra test. Maybe it's in the place I mostly copied this code from as well.

Thanks,
Richard.

>2013-01-22  Marek Polacek  <polacek@redhat.com>
>
>	* cfgloopmanip.c (fix_loop_structure): Remove redundant condition.
>
>	* testsuite/gcc.dg/pr56035.c: New test.
>
>--- gcc/cfgloopmanip.c.mp	2013-01-22 14:11:25.241233824 +0100
>+++ gcc/cfgloopmanip.c	2013-01-22 19:00:39.850689745 +0100
>@@ -1823,10 +1823,8 @@ fix_loop_structure (bitmap changed_bbs)
>       /* If there was no latch, schedule the loop for removal.  */
>       if (!first_latch)
> 	loop->header = NULL;
>-      /* If there was a single latch and it belongs to the loop of the
>-	 header, record it.  */
>-      else if (latch
>-	       && latch->src->loop_father == loop)
>+      /* If there was a single latch, record it.  */
>+      else if (latch)
> 	loop->latch = latch->src;
>       /* Otherwise there are multiple latches which are eventually
>          disambiguated below.  */
>--- gcc/testsuite/gcc.dg/pr56035.c.mp	2013-01-22 14:27:21.104614758
>+0100
>+++ gcc/testsuite/gcc.dg/pr56035.c	2013-01-22 14:31:01.642266091 +0100
>@@ -0,0 +1,35 @@
>+/* PR tree-optimization/56035 */
>+/* { dg-do compile } */
>+/* { dg-options "-O1 -ftree-vectorize -fcse-follow-jumps
>-fstrict-overflow" } */
>+
>+short a, c, *p;
>+
>+void
>+f (void)
>+{
>+  int b;
>+
>+  if (c)
>+  lbl1:
>+    for (a = 0; a < 1; a++)
>+      {
>+	for (c = 0; c < 1; c++)
>+	  {
>+	    goto lbl1;
>+	    while (*p++)
>+	    lbl2:
>+	      ;
>+	  }
>+      }
>+
>+  for (;; b++)
>+    {
>+      if (c)
>+	goto lbl2;
>+    lbl3:
>+      for (c = 0; c < 9; c++)
>+	for (c = -17; c < 2; c++)
>+	  if (*p)
>+	    goto lbl3;
>+    }
>+}
>
>	Marek