This is an archive of the discontinued LLVM Phabricator instance.

[LoopUnrollPeel] Add case where we should forget the peeled loop from SCEV.
ClosedPublic

Authored by fhahn on Feb 13 2019, 9:35 AM.

Details

Summary

The test case requires the peeled loop to be forgotten after peeling,
even though it does not have a parent. When called via the unroller,
SE->forgetTopmostLoop is also called, so the test case would also pass
without any SCEV invalidation, but peelLoop is exposed as utility
function. Also, in the test case, simplifyLoop will make changes,
removing the loop from SCEV, but it is better to not rely on this
behavior.

Diff Detail

Repository
rL LLVM

Event Timeline

fhahn created this revision.Feb 13 2019, 9:35 AM

LGTM, but use forgetTopmostLoop instead.

llvm/lib/Transforms/Utils/LoopUnrollPeel.cpp
668 ↗(On Diff #186689)

The correct way of doing this is L->forgetTopmostLoop(). Please us it to avoid potential sneaky bugs with containing loops in the future.

mkazantsev accepted this revision.Feb 13 2019, 9:11 PM
This revision is now accepted and ready to land.Feb 13 2019, 9:11 PM
fhahn updated this revision to Diff 186827.Feb 14 2019, 5:37 AM

Use forgetTopmostLoop and get rid of getting the parent loop, as it is not
required any longer.

fhahn added a comment.Feb 14 2019, 5:47 AM

Use forgetTopmostLoop and get rid of getting the parent loop, as it is not
required any longer.

I'll keep the code to get the parent loop for simplifyLoop when committing.

This revision was automatically updated to reflect the committed changes.