Page MenuHomePhabricator

[BasicBlockUtils] Check for unreachable preds before updating LI in UpdateAnalysisInformation
ClosedPublic

Authored by anna on Dec 21 2017, 2:31 PM.

Details

Summary

We are incorrectly updating the LI when loop-simplify generates
dedicated exit blocks for a loop. The issue is that there's an implicit
assumption that the Preds passed into UpdateAnalysisInformation are
reachable. However, this is not true and breaks LI by incorrectly
updating the header of a loop.

One such case is when we generate dedicated exits when the exit block is
a landing pad (through SplitLandingPadPredecessors). There maybe other
cases as well, since we do not guarantee that Preds passed in are
reachable basic blocks.

The added test case shows how loop-simplify breaks LI for the outer loop (and DT in turn)
after we try to generate the LoopSimplifyForm.

Event Timeline

anna created this revision.Dec 21 2017, 2:31 PM
davide requested changes to this revision.Dec 21 2017, 8:34 PM

I think this is on the right track, but, can you please simplify the test case so I can read it better :) ?

This revision now requires changes to proceed.Dec 21 2017, 8:34 PM
anna updated this revision to Diff 128016.Dec 22 2017, 4:48 AM

Simplified the testcase to make the issue clearer.

anna added a comment.Dec 22 2017, 4:59 AM

I think this is on the right track, but, can you please simplify the test case so I can read it better :) ?

Yes, absolutely agree. Actually, the testcase was a *manually simplified* version of bugpoint's output - bugpoint's simplified output had 230 lines in the testcase!
Clearly, the manual simplification (75 lines) could still be improved :)

As an aside I think bugpoint needs improvement around invokes and landing blocks (unless there's some specific option to use here other than -compile-custom).

davide accepted this revision.Jan 2 2018, 6:18 AM

LGTM, thanks Anna. (sorry for the delay, I was in Europe for the end of the year).

This revision is now accepted and ready to land.Jan 2 2018, 6:18 AM
anna added a comment.Jan 2 2018, 6:26 AM

LGTM, thanks Anna. (sorry for the delay, I was in Europe for the end of the year).

Thanks Davide! Happy new year :)

This revision was automatically updated to reflect the committed changes.