This is an archive of the discontinued LLVM Phabricator instance.

Don't try to rotate a loop more than once - we never do this anyway.
ClosedPublic

Authored by mzolotukhin on May 11 2016, 1:53 PM.

Details

Summary

I can't find a case where we can rotate a loop more than once, and it looks
like we never do this. To rotate a loop following conditions should be met:

  1. its header should be exiting
  2. its latch shouldn't be exiting

But after the first rotation the header becomes the new latch, so this
condition can never be true any longer.

Tested on with an assert on LNT testsuite and make check.

Diff Detail

Repository
rL LLVM

Event Timeline

mzolotukhin retitled this revision from to Don't try to rotate a loop more than once - we never do this anyway..
mzolotukhin updated this object.
mzolotukhin added a reviewer: hfinkel.
mzolotukhin added a subscriber: llvm-commits.

Is it possible that the new latch, after simplification, ends up being a non-exiting block? If not, might be useful to add an assert(NewLatch is exiting block).

lib/Transforms/Scalar/LoopRotation.cpp
572 ↗(On Diff #56960)

Fix comment?

585 ↗(On Diff #56960)

Fix comment?

Thanks for taking a look, Sanjoy!

Is it possible that the new latch, after simplification, ends up being a non-exiting block? If not, might be useful to add an assert(NewLatch is exiting block).

I haven't managed to find such case, but running testsuite with this assertion sounds like a good idea to me. I'll update the patch when the run finishes.

Thanks,
Michael

lib/Transforms/Scalar/LoopRotation.cpp
572 ↗(On Diff #56960)

Oops, thanks, will fix!

  • Add assert.
  • Fix comments.
mzolotukhin marked 3 inline comments as done.May 25 2016, 2:24 PM

Ping.

sanjoy accepted this revision.May 25 2016, 3:02 PM
sanjoy added a reviewer: sanjoy.

lgtm

This revision is now accepted and ready to land.May 25 2016, 3:02 PM
This revision was automatically updated to reflect the committed changes.