This is an archive of the discontinued LLVM Phabricator instance.

[LPM] Teach the legacy pass manager to support *using* an analysis without *requiring* it.
ClosedPublic

Authored by chandlerc on Aug 18 2015, 1:46 PM.

Details

Summary

This let's a pass indicate that it will use an analysis if available
(through getAnalysisIfAvailable). When the pass manager knows this, it
will refrain from deleting that analysis if it can. Naturally, it will
still get invalidated at the correct time. These passes are not
considered when scheduling the pass pipeline, so typically they will
require manual scheduling, but this may also allow passes with
getAnalysisIfAvailable to find the analysis more often if nothing after
them requires that analysis and it wasn't invalidated.

I don't have a particular use case with the current passes, but with my
new structure for alias analyses, this will be very useful. We want to
allow people to customize the set of AAs available by scheduling
additional passes. These's aren't ever *required* for obvious reasons.
So we need some way to mark in the legacy pass manager that they will
still be used if available.

This is essentially how analysis groups already work. But this makes the
feature generally available and more explicit. It should allow the AA
change to not impact how people trigger a custom alias analysis being
available at a certain point in compilation.

Diff Detail

Repository
rL LLVM

Event Timeline

chandlerc updated this revision to Diff 32446.Aug 18 2015, 1:46 PM
chandlerc retitled this revision from to [LPM] Teach the legacy pass manager to support *using* an analysis without *requiring* it..
chandlerc updated this object.
chandlerc added a subscriber: llvm-commits.

This seems really nice; if I understand correctly what this does, it fixes a long-standing bug (assuming that the passes themselves are updated). I recall writing workarounds in regression tests like this (in test/Transforms/InstCombine/assume-loop-align.ll):

; RUN: opt -domtree -instcombine -loops -S < %s | FileCheck %s
; Note: The -loops above can be anything that requires the domtree, and is
; necessary to work around a pass-manager bug.

(I don't know if we currently have a good example in a regression test because, since instcombine was changed to require a domtree, the workaround in this specific test should no longer be necessary).

include/llvm/PassAnalysisSupport.h
95 ↗(On Diff #32446)

Should we call this addUsedIfAvailable, or similar, to make it clearer what this does (and, specifically, how it differs from addRequired)?

chandlerc added inline comments.Aug 18 2015, 2:47 PM
include/llvm/PassAnalysisSupport.h
95 ↗(On Diff #32446)

Seems a small gain, but if you like. I don't really care. You (or others) give me a spelling.

hfinkel added inline comments.Aug 18 2015, 2:53 PM
include/llvm/PassAnalysisSupport.h
95 ↗(On Diff #32446)

Eh; many things changed during code review have small gains ;)

I think that addUsedIfAvailable goes well with getAnalysisIfAvailable. If anyone else has a preference, please speak up.

chandlerc accepted this revision.Aug 18 2015, 7:53 PM
chandlerc added a reviewer: chandlerc.
chandlerc marked 3 inline comments as done.

Thanks, submitting with the suggested change from Hal based on his feedback.

Feel free to grumble about better names in post-commit review if needed.

include/llvm/PassAnalysisSupport.h
95 ↗(On Diff #32446)

Done and submitting. Thanks!

This revision is now accepted and ready to land.Aug 18 2015, 7:53 PM
This revision was automatically updated to reflect the committed changes.
chandlerc marked an inline comment as done.