This is an archive of the discontinued LLVM Phabricator instance.

[LICM] Extend the MustExecute scope
ClosedPublic

Authored by skatkov on May 16 2018, 8:15 PM.

Details

Summary

CanProveNotTakenFirstIteration utility does not handle the case when
condition of the branch is a constant. Add its handling.

Diff Detail

Repository
rL LLVM

Event Timeline

skatkov created this revision.May 16 2018, 8:15 PM
reames accepted this revision.May 17 2018, 9:05 AM

LGTM w/minor changes

Separately, Serguei, you should try to figure out what the pass ordering problem in our downstream pipeline was that we don't simplify the branches away before we get to LICM. This patch is definitely useful though, so that's an orthogonal improvement to pursue.

test/Analysis/MustExecute/const-cond.ll
4 ↗(On Diff #147231)

Can you add a comment above to indicate the value of this w.r.t. eliminating pass ordering dependencies?

test/Transforms/LICM/hoist-mustexec.ll
221 ↗(On Diff #147231)

This isn't the intent of this test. Can you tweak the test to ensure this keeps testing the negative case?

This revision is now accepted and ready to land.May 17 2018, 9:05 AM
skatkov updated this revision to Diff 147432.May 17 2018, 8:29 PM

Before landing.

Philip, I hope I correctly understood the intent of the test.

Seems reasonable, go for it.

This revision was automatically updated to reflect the committed changes.