This is an archive of the discontinued LLVM Phabricator instance.

[LoopUnswitch] Unswitch on conditions feeding into guards
ClosedPublic

Authored by sanjoy on Jun 25 2016, 12:52 AM.

Details

Summary

This is a straightforward extension of what LoopUnswitch does to
branches to guards. That is, we unswitch

for (;;) {
  ...
  guard(loop_invariant_cond);
  ...
}

into

if (loop_invariant_cond) {
  for (;;) {
    ...
    // There is no need to emit guard(true)
    ...
  }
} else {
  for (;;) {
    ...
    guard(false);
    // SimplifyCFG will clean this up by adding an
    // unreachable after the guard(false)
    ...
  }
}

Diff Detail

Event Timeline

sanjoy updated this revision to Diff 61887.Jun 25 2016, 12:52 AM
sanjoy retitled this revision from to [LoopUnswitch] Unswitch on conditions feeding into guards.
sanjoy updated this object.
sanjoy added a reviewer: majnemer.
sanjoy added a subscriber: llvm-commits.
majnemer added inline comments.Jun 25 2016, 1:17 AM
lib/Transforms/Scalar/LoopUnswitch.cpp
518–533

For some reason my eye is hunting for punctuation, maybe throw on a ':'

526

Perhaps "That loops don't contain ..."

527

.

532

Is it safe to make this an AssertingVH? Could unswitching one guard lead to another guard getting cloned while the original gets erased?

sanjoy updated this revision to Diff 61888.Jun 25 2016, 1:40 AM
sanjoy marked 4 inline comments as done.
  • address review
lib/Transforms/Scalar/LoopUnswitch.cpp
532

Good point. Added a test case and changed to keep raw pointers for now.

majnemer added inline comments.Jun 25 2016, 1:43 AM
lib/Transforms/Scalar/LoopUnswitch.cpp
532

Hmm, does switching to a raw pointer help? In the even in which the guard was cloned and destroyed, the guard in your list would be a bad pointer.

sanjoy updated this revision to Diff 61897.Jun 25 2016, 12:27 PM
sanjoy marked an inline comment as done.
  • Add comment
lib/Transforms/Scalar/LoopUnswitch.cpp
532

My reading of the code is that that's fine, since if we do invalidate something in Guards, we'll return and never look at Guards again. I've added a comment for that.

majnemer accepted this revision.Jun 25 2016, 9:47 PM
majnemer edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 25 2016, 9:47 PM
This revision was automatically updated to reflect the committed changes.