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

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

Details

Summary

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

Repository
rL LLVM
craig.topper created this revision.Tue, Nov 14, 9:43 AM

Now with test case.

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.Tue, Nov 14, 3:10 PM

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

test/Transforms/LoopRotate/pr35210.ll
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.Tue, Nov 14, 3:14 PM

Other than that, lg.

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