Comments

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