On Wed, Jan 4, 2012 at 2:01 PM, Jan Hubicka <hubi...@ucw.cz> wrote:
> Hi,
> in the testcase bellow the loop peeling simplifies exit condition in a way
> that the outer loop is destroyed. After some discussion with Zdenek I learnt
> that remove_path is then supposed to destroy the outer loop by calling unloop
> on it, but there is thinko in the function assuming that just the immediately
> outer loop can be eliminated. This is not true in this case: inner loop
> has two exists, one closing outer loop and the ohter closing outer loop of the
> outer loop. Outer loop stays, but outer loop of outer loop needs to be
> removed,
> so we need to walk the whole hiarchy upwards.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
> int a, b, c, d;
>
> static void
> foo (int *x)
> {
> c = 0;
> while (1)
> {
> if (*x)
> break;
> while (b)
> for (; c; c = 0);
> for (d = 18; d != 18; d++)
> if (c)
> {
> foo (x);
> return;
> }
> }
> }
>
> static void
> bar ()
> {
> foo (0);
> foo (0);
> for (;;)
> ;
> }
>
> baz ()
> {
> for (; a;)
> bar ();
> }
> PR middle-end/49710
> * cfgloopmanip.c (remove_path): Walk loop hiearchy upwards when
> unlooping loops.
> Index: cfgloopmanip.c
> ===================================================================
> *** cfgloopmanip.c (revision 182871)
> --- cfgloopmanip.c (working copy)
> *************** remove_path (edge e)
> *** 291,296 ****
> --- 291,297 ----
> sbitmap seen;
> bool irred_invalidated = false;
> edge_iterator ei;
> + struct loop *l;
>
> if (!can_remove_branch_p (e))
> return false;
> *************** remove_path (edge e)
> *** 315,323 ****
> normally. We may assume that e->dest is not a header of any loop,
> as it now has exactly one predecessor. */
> while (loop_outer (e->src->loop_father)
> ! && dominated_by_p (CDI_DOMINATORS,
> ! e->src->loop_father->latch, e->dest))
> unloop (e->src->loop_father, &irred_invalidated);
>
> /* Identify the path. */
> nrem = find_path (e, &rem_bbs);
> --- 316,333 ----
> normally. We may assume that e->dest is not a header of any loop,
> as it now has exactly one predecessor. */
> while (loop_outer (e->src->loop_father)
> ! && dominated_by_p (CDI_DOMINATORS,
> ! e->src->loop_father->latch, e->dest))
> unloop (e->src->loop_father, &irred_invalidated);
> + l = e->src->loop_father;
> + while (l && loop_outer (l))
> + {
> + while (loop_outer (loop_outer (l))
> + && dominated_by_p (CDI_DOMINATORS,
> + loop_outer (l)->latch, e->dest))
> + unloop (loop_outer (l), &irred_invalidated);
> + l = loop_outer (l);
> + }
It must be possible to merge the two loops, no? And it asks for a
comment.
> /* Identify the path. */
> nrem = find_path (e, &rem_bbs);
