This is an archive of the discontinued LLVM Phabricator instance.

[LoopInterchange] Turn into a loop pass.
ClosedPublic

Authored by fhahn on Sep 5 2018, 12:10 PM.

Details

Summary

This patch turns LoopInterchange into a loop pass. It now only
considers top-level loops and tries to move the innermost loop to the
optimal position within the loop nest. By only looking at top-level
loops, we might miss a few opportunities the function pass would get
(e.g. if we have a loop nest of 3 loops, in the function pass
we might process loops at level 1 and 2 and move the inner most loop to
level 1, and then we process loops at levels 0, 1, 2 and interchange
again, because we now have a different inner loop). But I think it would
be better to handle such cases by picking the best inner loop from the
start and avoid re-visiting the same loops again.

The biggest advantage of it being a function pass is that it interacts
nicely with the other loop passes. Without this patch, there are some
performance regressions on AArch64 with loop interchanging enabled,
where no loops were interchanged, but we missed out on some other loop
optimizations.

It also removes the SimplifyCFG run. We are just changing branches, so
the CFG should not be more complicated, besides the additional 'unique'
preheaders this pass might create.

Diff Detail

Repository
rL LLVM

Event Timeline

fhahn created this revision.Sep 5 2018, 12:10 PM
fhahn added inline comments.Sep 5 2018, 12:10 PM
lib/Transforms/Scalar/LoopInterchange.cpp
434 ↗(On Diff #164097)

Not needed, I'll drop it.

fhahn updated this revision to Diff 164201.EditedSep 6 2018, 6:14 AM

Rebased after rL341537

xbolva00 accepted this revision.EditedSep 11 2018, 6:08 AM
xbolva00 added a subscriber: xbolva00.

Thanks, looks great as LoopPass.

This revision is now accepted and ready to land.Sep 11 2018, 6:08 AM
fhahn added a comment.Sep 13 2018, 9:44 AM

Further testing surfaced a few cases where ScalarEvolution and LCSSA are not preserved properly. I'll fix those issues first.

fhahn updated this revision to Diff 167177.Sep 26 2018, 12:54 PM

Rebased after the required fixes went in. Unless there are any additional concerns, I plan to go ahead and commit this tomorrow.

This revision was automatically updated to reflect the committed changes.