On Tue, Jan 22, 2013 at 05:26:02PM +0100, Zdenek Dvorak wrote:
> 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). 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.
Marek
