This is an archive of the discontinued LLVM Phabricator instance.

[LoopRotate] processLoop should return true even if it just simplified the loop latch without making any other changes

Authored by craig.topper on Nov 14 2017, 9:43 AM.



Simplifying a loop latch changes the IR and we need to make sure the pass manager knows to invalidate analysis passes if that happened.

PR35210 discovered a case where we failed to invalidate the post dominator tree after this simplification because we no changes other than simplifying the loop latch.

I'm still trying to put together a reduced test that triggers only the latch simplification, but I don't know anything about this pass so if anyone wants to help that would be great.

Fixes PR35210.

Diff Detail


Event Timeline

craig.topper created this revision.Nov 14 2017, 9:43 AM

Now with test case.

efriedma edited edge metadata.Nov 14 2017, 2:52 PM

Someone more familiar with the new pass manager stuff should review the CHECK lines; not sure what the recommended way to write a pass invalidation test is.

Otherwise LGTM.

davide added a subscriber: davide.Nov 14 2017, 3:10 PM

I'm under the impression the fix is correct, a couple of comments inline.

1 ↗(On Diff #122888)

I don't remember this exactly, but DEBUG implies REQUIRES: assert?

17 ↗(On Diff #122888)

Please make sure this passes on Windows, because there have been problems (you might want to use just <*> as other tests do).

davide accepted this revision.Nov 14 2017, 3:14 PM

Other than that, lg.

This revision is now accepted and ready to land.Nov 14 2017, 3:14 PM
This revision was automatically updated to reflect the committed changes.