This is an archive of the discontinued LLVM Phabricator instance.

[LoopInterchange] Fix transformation bugs in loop interchange
ClosedPublic

Authored by congzhe on Mar 11 2021, 8:09 PM.

Details

Summary

After loop interchange, the (old) outer loop header should not jump to
the LoopExit. Note that the old outer loop becomes the new inner loop
after interchange. If we branched to LoopExit then after interchange
we would jump directly from the (new) inner loop header to LoopExit
without executing the rest of outer loop.

This patch modifies adjustLoopBranches() such that the old outer
loop header (which becomes the new inner loop header) jumps to the
old inner loop latch which becomes the new outer loop latch after
interchange.

Diff Detail

Event Timeline

congzhe created this revision.Mar 11 2021, 8:09 PM
congzhe requested review of this revision.Mar 11 2021, 8:09 PM
congzhe edited the summary of this revision. (Show Details)Mar 11 2021, 8:10 PM
This comment was removed by congzhe.
congzhe updated this revision to Diff 335572.Apr 6 2021, 10:08 AM

Removed unnecessary code, simplified the logic.

Thanks for fixing this problem. I just have some comments about the tests, otherwise LGTM.

llvm/test/Transforms/LoopInterchange/outer-header-jump-to-inner-latch.ll
6

[nit] unused, please remove it.

9

[nit] unused, please remove it.

13

Could you please put the C example here as a comment.

91

Please put the C example as a comment and mention which loops get interchanged for better readability.

137

could you please rename these labels to make them more meaningful?

bmahjour added inline comments.Apr 6 2021, 1:46 PM
llvm/test/Transforms/LoopInterchange/outer-header-jump-to-inner-latch.ll
92

[nit] remove unused attributes and markers like fastcc, unnamed_addr, #1

congzhe updated this revision to Diff 335844.Apr 7 2021, 9:36 AM

All comments addressed.

Thanks for fixing this problem. I just have some comments about the tests, otherwise LGTM.

Thanks for the review! All comments are now addressed.

  1. Removed unused items in the test case.
  2. Provided the corresponding C test examples.
  3. Updated variable names and labels in tests for better readability.
This revision is now accepted and ready to land.Apr 7 2021, 11:23 AM
congzhe updated this revision to Diff 336134.Apr 8 2021, 8:51 AM

Removed the variable LoopExit.

I committed and reverted the patch since it fails x86 sanitizer check. The reason is that after the patch, the variable LoopExit becomes unused.

Now I removed LoopExit, I'd appreciate if you could take a look @bmahjour, thanks a lot!

congzhe reopened this revision.Apr 8 2021, 8:55 AM
This revision is now accepted and ready to land.Apr 8 2021, 8:55 AM

I committed and reverted the patch since it fails x86 sanitizer check. The reason is that after the patch, the variable LoopExit becomes unused.

Now I removed LoopExit, I'd appreciate if you could take a look @bmahjour, thanks a lot!

Seems fine to me.

This revision was landed with ongoing or failed builds.Apr 8 2021, 11:59 AM
This revision was automatically updated to reflect the committed changes.