This is an archive of the discontinued LLVM Phabricator instance.

[PM] Move the LoopPassManager to the transforms library.
ClosedPublic

Authored by chandlerc on Jan 7 2017, 7:52 PM.

Details

Summary

While the loop PM uses an analysis to form the IR units, the current
plan is to have the PM itself establish and enforce both loop simplified
form and LCSSA. This would be a layering violation in the analysis
library.

Fundamentally, the idea behind the loop PM is to *transform* loops in
addition to running passes over them, so it really seemed like the most
natural place to sink this was into the transforms library.

Event Timeline

chandlerc updated this revision to Diff 83545.Jan 7 2017, 7:52 PM
chandlerc retitled this revision from to [PM] Move the LoopPassManager to the transforms library..
chandlerc updated this object.
chandlerc added a subscriber: llvm-commits.
mehdi_amini accepted this revision.Jan 7 2017, 7:57 PM
mehdi_amini edited edge metadata.

While surprising at first that makes the most sense after thinking a bit about it.

This revision is now accepted and ready to land.Jan 7 2017, 7:57 PM
sanjoy accepted this revision.Jan 7 2017, 8:32 PM
sanjoy edited edge metadata.

Any reason to prefer Transforms/Scalar over Transforms/Utils? I'd have expected the latter.

Also: don't forget to sort the #include s before checkin.

Any reason to prefer Transforms/Scalar over Transforms/Utils? I'd have expected the latter.

I believe it'll needs to call into LCSAA/LoopSimplify, are these in Utils?

Any reason to prefer Transforms/Scalar over Transforms/Utils? I'd have expected the latter.

Because Utils is mostly non-passes. I'd like eventually to make that a reality.

And I'd like the loop PM to be able to use the *pass* formulations of LCSSA and LoopSimplify so that it can work (and be debugged) very much as a less customizable pass pipeline. Does that make sense?

sanjoy added a comment.Jan 7 2017, 9:29 PM

Any reason to prefer Transforms/Scalar over Transforms/Utils? I'd have expected the latter.

Because Utils is mostly non-passes. I'd like eventually to make that a reality.

And I'd like the loop PM to be able to use the *pass* formulations of LCSSA and LoopSimplify so that it can work (and be debugged) very much as a less customizable pass pipeline. Does that make sense?

sgtm!

chandlerc updated this revision to Diff 83848.Jan 10 2017, 12:07 PM
chandlerc edited edge metadata.

Really substantial change -- now we *split* the loop PM into the analysis
management and transformation aware components. This is necessary to avoid
breaking the layering of the analysis library.

jlebar accepted this revision.Jan 10 2017, 4:06 PM
jlebar added a reviewer: jlebar.
jlebar added a subscriber: jlebar.

Looks fine to me.

include/llvm/Analysis/LoopAnalysisManager.h
18 ↗(On Diff #83848)

s/loop-specific// (DominatorTree, SCEV, AAManager aren't loop-specific).

50 ↗(On Diff #83848)

s/standard//

52 ↗(On Diff #83848)

Suggest

The adaptor from a function pass to a loop pass computes these analyses and makes them available to the loop passes "for free". Each loop pass is expected expected to update these analyses if necessary to ensure they're valid after it runs.

67 ↗(On Diff #83848)

nit, newline after?

chandlerc marked 3 inline comments as done.Jan 10 2017, 10:42 PM

Thanks, landing!

include/llvm/Analysis/LoopAnalysisManager.h
67 ↗(On Diff #83848)

I've been doing this everywhere. Lemme send you a separate patch that tries a different and less confusing (i hope) pattern.

This revision was automatically updated to reflect the committed changes.
include/llvm/Transforms/Scalar/LoopInstSimplify.h