Page MenuHomePhabricator

[IndVars] Fix PR26974: make sure replaceCongruentIVs doesn't break LCSSA
ClosedPublic

Authored by sbaranga on Mar 18 2016, 7:25 AM.

Details

Summary

replaceCongruentIVs can break LCSSA when trying to replace IV increments
since it tries to replace all uses of a phi node with another phi node
while both of the phi nodes are not necessarily in the processed loop.
This will cause an assert in IndVars.

To fix this, we add a check to make sure that the replacement maintains
LCSSA.

Diff Detail

Event Timeline

sbaranga updated this revision to Diff 51022.Mar 18 2016, 7:25 AM
sbaranga retitled this revision from to [IndVars] Fix PR26974: make sure replaceCongruentIVs doesn't break LCSSA.
sbaranga updated this object.
sbaranga added a reviewer: sanjoy.
sbaranga added a subscriber: llvm-commits.
sanjoy added inline comments.Mar 18 2016, 11:07 AM
test/Transforms/IndVarSimplify/rewrite-iv-lcssa.ll
3 ↗(On Diff #51022)

Normally, I see tests put the ; RUN line at the very top.

Also, since this is fixing a PR, the convention I've generally observed is to call the test file prXXXX.ll.

sanjoy accepted this revision.Mar 18 2016, 11:16 AM
sanjoy edited edge metadata.

Other than that, lgtm

This revision is now accepted and ready to land.Mar 18 2016, 11:16 AM
sbaranga updated this revision to Diff 51152.Mar 21 2016, 5:43 AM
sbaranga edited edge metadata.

Move the regeression test to pr26974.ll and make sure the RUN line is first in the test.

sbaranga closed this revision.Mar 21 2016, 5:49 AM

Thanks, Sanjoy! I've made the changes and committed in r263941.