Hi,

> > > We ICEd on attached testcase because we didn't properly detected the
> > > latch edge.  Furthermore, I think the code for re-computing latches
> > > is somehow broken at the moment.  In fix_loop_structure we have
> > >       /* 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)
> > >   loop->latch = latch->src;
> > >       /* Otherwise there are multiple latches which are eventually
> > >          disambiguated below.  */
> > >       else
> > >   loop->latch = NULL;
> > > but I don't think we can use the ->loop_father info, because that is
> > > already cleared by
> > >   FOR_EACH_BB (bb)
> > >     {
> > >       if (changed_bbs)
> > >   bb->aux = (void *) (size_t) loop_depth (bb->loop_father);
> > >       bb->loop_father = current_loops->tree_root;
> > >     }
> > > earlier on.
> > 
> > I am not quite sure why we test "&& latch->src->loop_father == loop" in
> > the first place.  Would things work if that condition is removed?
> > I don't much like your proposed patch (as well as the current state),
> > as according to the specification of fix_loop_structure, we are not
> > allowed to assume anything about the bb --> loop mapping,
> 
> I tried that approach, too, and it worked (even regtested that
> successfully).

let's go with that, then (as well as updating the missleading comment before 
the test).

> 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),

Zdenek

Reply via email to