Page MenuHomePhabricator

[LoopUnroll] Use addClonedBlockToLoopInfo to clone the top level loop (NFC)
ClosedPublic

Authored by fhahn on Jan 26 2017, 5:02 AM.

Details

Summary

rL293124 added the necessary infrastructure to properly add the cloned
top level loop to LoopInfo, which means we do not have to do it manually
in CloneLoopBlocks.

@mkuper sorry for not pointing this out during my review of D29156, I just
realized that today.

Diff Detail

Event Timeline

fhahn created this revision.Jan 26 2017, 5:02 AM
fhahn edited the summary of this revision. (Show Details)
fhahn updated this revision to Diff 85889.Jan 26 2017, 5:12 AM

Put NewLoop decl in if.

mkuper edited edge metadata.Jan 26 2017, 2:44 PM

Nice cleanup, thanks - addClonedBlockToLoopInfo() will indeed add the top-level itself, I didn't notice this when I wrote the patch.
But I think you still need to know whether you actually created a remainder loop, see inline comment.

lib/Transforms/Utils/LoopUnrollRuntime.cpp
390

Are you sure you want to do this if NewLoops[L] is the parent loop? I think this code is meant to only disable unrolling on the remainder loop.

fhahn updated this revision to Diff 85986.Jan 26 2017, 4:13 PM

Fixed check

lib/Transforms/Utils/LoopUnrollRuntime.cpp
390

Ah good spot. We should check CreateRemainderLoop I think.

mkuper accepted this revision.Jan 30 2017, 3:32 PM

Sorry, I missed the fixed revision.
LGTM.

This revision is now accepted and ready to land.Jan 30 2017, 3:32 PM
fhahn closed this revision.Jan 31 2017, 3:24 AM