Page MenuHomePhabricator

[Loop Peeling] Fix the bug with IDom setting for exit loops
ClosedPublic

Authored by skatkov on Jul 12 2019, 1:49 AM.

Details

Summary

It is possible that loop exit has two predecessors in a loop body.
In this case after the peeling the iDom of the exit should be a clone of
iDom of original exit but no a clone of a block coming to this exit.

Diff Detail

Repository
rL LLVM

Event Timeline

skatkov created this revision.Jul 12 2019, 1:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2019, 1:49 AM
reames accepted this revision.Jul 12 2019, 10:42 AM

LGTM. An alternate implementation would be to require that each exit has one predecessor in the legality for peeling check. Either would be fine, and if you want to do the alternate, you can land that without review..

llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
588 ↗(On Diff #209428)

Please add an assert that L has dedicated exits, because that's the precondition for your dominance fact.

This revision is now accepted and ready to land.Jul 12 2019, 10:42 AM
skatkov marked an inline comment as done.Jul 14 2019, 10:06 PM
skatkov added inline comments.
llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
588 ↗(On Diff #209428)

In the beginning of this method we have an assert that we canPeel which returns true only if loop is in simplify form what includes the check that all exits are dedicated.

It is to far so I'm ok to add an assert here as well.

This revision was automatically updated to reflect the committed changes.