This is an archive of the discontinued LLVM Phabricator instance.

[LPM] Factor all of the loop analysis usage updates into a common helper routine.
ClosedPublic

Authored by chandlerc on Feb 18 2016, 8:18 PM.

Details

Summary

We were getting this wrong in small ways and generally being very
inconsistent about it across loop passes. Instead, let's have a common
place where we do this. One minor downside is that this will require
some analyses like SCEV in more places than they are strictly needed.
However, this seems benign as these analyses are complete no-ops, and
without this consistency we can in many cases end up with the legacy
pass manager scheduling deciding to split up a loop pass pipeline in
order to run the function analysis half-way through. It is very, very
annoying to fix these without just being very pedantic across the board.

The only loop passes I've not updated here are ones that use
AU.setPreservesAll() such as IVUsers (an analysis) and the pass printer.
They seemed less relevant.

With this patch, almost all of the problems in PR24804 around loop pass
pipelines are fixed. The one remaining issue is that we run simplify-cfg
and instcombine in the middle of the loop pass pipeline. We've recently
added some loop variants of these passes that would seem substantially
cleaner to use, but this at least gets us much closer to the previous
state. Notably, the seven loop pass managers is down to three.

Note that this doesn't currently pass tests because in changing this, I've made
the INITIALIZE_PASS_DEPENDENCY nonsense totally not work. I'll fix that, but
that won't change the substantive parts of this patch that I'd really like to
get review feedback on -- essentially, is this a reasonable approach?

Diff Detail

Event Timeline

chandlerc updated this revision to Diff 48451.Feb 18 2016, 8:18 PM
chandlerc retitled this revision from to [LPM] Factor all of the loop analysis usage updates into a common helper routine..
chandlerc updated this object.
chandlerc added a reviewer: hfinkel.
chandlerc added a subscriber: llvm-commits.
jmolloy accepted this revision.Feb 19 2016, 1:41 AM
jmolloy added a reviewer: jmolloy.
jmolloy added a subscriber: jmolloy.

This LGTM.

This revision is now accepted and ready to land.Feb 19 2016, 1:41 AM
chandlerc updated this revision to Diff 48466.Feb 19 2016, 2:12 AM
chandlerc edited edge metadata.

Update the patch with tests, and to remove changes to passes that doen't
actually need them. This now passes all tests while reducing the pass pipelines
as desired.

Small comment typos.

also, should we add this to WritingAnLLVMPass.rst?

lib/Transforms/Utils/LoopUtils.cpp
771

generic. dependency.

Thanks, landing based on confirmed LGTM on IRC.

Small comment typos.

Fixed.

also, should we add this to WritingAnLLVMPass.rst?

Follow-up patch will be forthcoming.

This revision was automatically updated to reflect the committed changes.