Page MenuHomePhabricator

Expand cloneLoopWithPreheader() to support cloning loop nest
ClosedPublic

Authored by Whitney on Jun 17 2019, 10:56 AM.

Details

Summary

cloneLoopWithPreheader() currently only support innermost loop, and assert otherwise.

Diff Detail

Event Timeline

Whitney created this revision.Jun 17 2019, 10:56 AM
Meinersbur added inline comments.Jun 17 2019, 12:35 PM
llvm/lib/Transforms/Utils/CloneFunction.cpp
744

Consider using DenseMap.

771

[style] I like getting a reference that can be updated instead of another map lookup. I.e.

Loop *&NewLoop = LMap[CurLoop];
...
NewLoop = LI->AllocateLoop();
776–777

[serious] Since the iteration order of OrigLoop->getBlocks() is undefined, we might get a block that is nested in two loops without having visited the parent first, i.e. NewParentLoop would be null in this case.

Consider iterating over LoopInfo's loop tree before iterating over all block to ensure that the new loop structure has been created.

Whitney updated this revision to Diff 205171.Jun 17 2019, 1:40 PM

Addressed all review comments from Michael.

Whitney marked 3 inline comments as done.Jun 17 2019, 1:41 PM
Whitney added inline comments.
llvm/lib/Transforms/Utils/CloneFunction.cpp
776–777

Good catch! Modified as suggested.

@Meinersbur Thanks for the review! What do you think of the updated changeset?

This revision is now accepted and ready to land.Jun 25 2019, 3:50 AM
This revision was automatically updated to reflect the committed changes.