Skip to content

Commit

Permalink
[NewPassManager] Add assertions when getting statefull cached analysis.
Browse files Browse the repository at this point in the history 
Summary:
Analyses that are statefull should not be retrieved through a proxy from
an outer IR unit, as these analyses are only invalidated at the end of
the inner IR unit manager.
This patch disallows getting the outer manager and provides an API to
get a cached analysis through the proxy. If the analysis is not
stateless, the call to getCachedResult will assert.

Reviewers: chandlerc

Subscribers: mehdi_amini, eraman, hiraditya, zzheng, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D72893
  • Loading branch information
@alinas
alinas committed May 13, 2020
1 parent db04ff4 commit bd541b2
Show file tree
Hide file tree
Showing 19 changed files with 292 additions and 204 deletions.
3 changes: 0 additions & 3 deletions clang/test/CodeGen/thinlto-distributed-newpm.ll
View file
Expand Up @@ -85,10 +85,7 @@
; CHECK-O: Running pass: PostOrderFunctionAttrsPass on (main)
; CHECK-O: Invalidating all non-preserved analyses for: (main)
; CHECK-O: Clearing all analysis results for: main
; CHECK-O: Invalidating analysis: FunctionAnalysisManagerCGSCCProxy on (main)
; CHECK-O3: Running pass: ArgumentPromotionPass on (main)
; CHECK-O2: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PassManager{{.*}}>
; CHECK-O: Running analysis: FunctionAnalysisManagerCGSCCProxy on (main)
; CHECK-O3: Running analysis: TargetIRAnalysis on main
; CHECK-O: Running analysis: PassInstrumentationAnalysis on main
; CHECK-O3: Running pass: CGSCCToFunctionPassAdaptor<{{.*}}PassManager{{.*}}>
Expand Down
4 changes: 2 additions & 2 deletions llvm/include/llvm/Analysis/AliasAnalysis.h
View file
Expand Up @@ -1185,8 +1185,8 @@ class AAManager : public AnalysisInfoMixin<AAManager> {
static void getModuleAAResultImpl(Function &F, FunctionAnalysisManager &AM,
AAResults &AAResults) {
auto &MAMProxy = AM.getResult<ModuleAnalysisManagerFunctionProxy>(F);
auto &MAM = MAMProxy.getManager();
if (auto *R = MAM.template getCachedResult<AnalysisT>(*F.getParent())) {
if (auto *R =
MAMProxy.template getCachedResult<AnalysisT>(*F.getParent())) {
AAResults.addAAResult(*R);
MAMProxy
.template registerOuterAnalysisInvalidation<AnalysisT, AAManager>();
Expand Down
38 changes: 29 additions & 9 deletions llvm/include/llvm/Analysis/CGSCCPassManager.h
View file
Expand Up @@ -380,10 +380,15 @@ class FunctionAnalysisManagerCGSCCProxy
public:
class Result {
public:
explicit Result() : FAM(nullptr) {}
explicit Result(FunctionAnalysisManager &FAM) : FAM(&FAM) {}

void updateFAM(FunctionAnalysisManager &FAM) { this->FAM = &FAM; }
/// Accessor for the analysis manager.
FunctionAnalysisManager &getManager() { return *FAM; }
FunctionAnalysisManager &getManager() {
assert(FAM);
return *FAM;
}

bool invalidate(LazyCallGraph::SCC &C, const PreservedAnalyses &PA,
CGSCCAnalysisManager::Invalidator &Inv);
Expand Down Expand Up @@ -415,7 +420,8 @@ using CGSCCAnalysisManagerFunctionProxy =
/// update result struct for the overall CGSCC walk.
LazyCallGraph::SCC &updateCGAndAnalysisManagerForFunctionPass(
LazyCallGraph &G, LazyCallGraph::SCC &C, LazyCallGraph::Node &N,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR);
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
FunctionAnalysisManager &FAM);

/// Helper to update the call graph after running a CGSCC pass.
///
Expand All @@ -425,7 +431,8 @@ LazyCallGraph::SCC &updateCGAndAnalysisManagerForFunctionPass(
/// update result struct for the overall CGSCC walk.
LazyCallGraph::SCC &updateCGAndAnalysisManagerForCGSCCPass(
LazyCallGraph &G, LazyCallGraph::SCC &C, LazyCallGraph::Node &N,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR);
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
FunctionAnalysisManager &FAM);

/// Adaptor that maps from a SCC to its functions.
///
Expand Down Expand Up @@ -516,7 +523,7 @@ class CGSCCToFunctionPassAdaptor
auto PAC = PA.getChecker<LazyCallGraphAnalysis>();
if (!PAC.preserved() && !PAC.preservedSet<AllAnalysesOn<Module>>()) {
CurrentC = &updateCGAndAnalysisManagerForFunctionPass(CG, *CurrentC, *N,
AM, UR);
AM, UR, FAM);
assert(
CG.lookupSCC(*N) == CurrentC &&
"Current SCC not updated to the SCC containing the current node!");
Expand Down Expand Up @@ -719,6 +726,7 @@ class DevirtSCCRepeatedPass
// Update the analysis manager with each run and intersect the total set
// of preserved analyses so we're ready to iterate.
AM.invalidate(*C, PassPA);

PA.intersect(std::move(PassPA));
}

Expand Down Expand Up @@ -754,6 +762,10 @@ ModuleToPostOrderCGSCCPassAdaptor<CGSCCPassT>::run(Module &M,
// Get the call graph for this module.
LazyCallGraph &CG = AM.getResult<LazyCallGraphAnalysis>(M);

// Get Function analysis manager from its proxy.
FunctionAnalysisManager &FAM =
AM.getCachedResult<FunctionAnalysisManagerModuleProxy>(M)->getManager();

// We keep worklists to allow us to push more work onto the pass manager as
// the passes are run.
SmallPriorityWorklist<LazyCallGraph::RefSCC *, 1> RCWorklist;
Expand Down Expand Up @@ -829,11 +841,12 @@ ModuleToPostOrderCGSCCPassAdaptor<CGSCCPassT>::run(Module &M,
continue;
}

// Ensure we can proxy analysis updates from from the CGSCC analysis
// manager into the Function analysis manager by getting a proxy here.
// FIXME: This seems like a bit of a hack. We should find a cleaner
// or more costructive way to ensure this happens.
(void)CGAM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, CG);
// Ensure we can proxy analysis updates from the CGSCC analysis manager
// into the the Function analysis manager by getting a proxy here.
// This also needs to update the FunctionAnalysisManager, as this may be
// the first time we see this SCC.
CGAM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, CG).updateFAM(
FAM);

// Each time we visit a new SCC pulled off the worklist,
// a transformation of a child SCC may have also modified this parent
Expand Down Expand Up @@ -888,6 +901,13 @@ ModuleToPostOrderCGSCCPassAdaptor<CGSCCPassT>::run(Module &M,
C = UR.UpdatedC ? UR.UpdatedC : C;
RC = UR.UpdatedRC ? UR.UpdatedRC : RC;

if (UR.UpdatedC) {
// If we're updating the SCC, also update the FAM inside the proxy's
// result.
CGAM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, CG).updateFAM(
FAM);
}

// If the CGSCC pass wasn't able to provide a valid updated SCC,
// the current SCC may simply need to be skipped if invalid.
if (UR.InvalidatedSCCs.count(C)) {
Expand Down
29 changes: 28 additions & 1 deletion llvm/include/llvm/IR/PassManager.h
View file
Expand Up @@ -799,6 +799,16 @@ template <typename IRUnitT, typename... ExtraArgTs> class AnalysisManager {
return &static_cast<ResultModelT *>(ResultConcept)->Result;
}

/// Verify that the given Result cannot be invalidated, assert otherwise.
template <typename PassT>
void verifyNotInvalidated(IRUnitT &IR, typename PassT::Result *Result) const {
PreservedAnalyses PA = PreservedAnalyses::none();
SmallDenseMap<AnalysisKey *, bool, 8> IsResultInvalidated;
Invalidator Inv(IsResultInvalidated, AnalysisResults);
assert(!Result->invalidate(IR, PA, Inv) &&
"Cached result cannot be invalidated");
}

/// Register an analysis pass with the manager.
///
/// The parameter is a callable whose result is an analysis pass. This allows
Expand Down Expand Up @@ -1064,7 +1074,24 @@ class OuterAnalysisManagerProxy
public:
explicit Result(const AnalysisManagerT &OuterAM) : OuterAM(&OuterAM) {}

const AnalysisManagerT &getManager() const { return *OuterAM; }
/// Get a cached analysis. If the analysis can be invalidated, this will
/// assert.
template <typename PassT, typename IRUnitTParam>
typename PassT::Result *getCachedResult(IRUnitTParam &IR) const {
typename PassT::Result *Res =
OuterAM->template getCachedResult<PassT>(IR);
if (Res)
OuterAM->template verifyNotInvalidated<PassT>(IR, Res);
return Res;
}

/// Method provided for unit testing, not intended for general use.
template <typename PassT, typename IRUnitTParam>
bool cachedResultExists(IRUnitTParam &IR) const {
typename PassT::Result *Res =
OuterAM->template getCachedResult<PassT>(IR);
return Res != nullptr;
}

/// When invalidation occurs, remove any registered invalidation events.
bool invalidate(
Expand Down
3 changes: 3 additions & 0 deletions llvm/include/llvm/Transforms/Utils/CallGraphUpdater.h
View file
Expand Up @@ -49,6 +49,7 @@ class CallGraphUpdater {
LazyCallGraph::SCC *SCC = nullptr;
CGSCCAnalysisManager *AM = nullptr;
CGSCCUpdateResult *UR = nullptr;
FunctionAnalysisManager *FAM = nullptr;
///}

public:
Expand All @@ -68,6 +69,8 @@ class CallGraphUpdater {
this->SCC = &SCC;
this->AM = &AM;
this->UR = &UR;
FAM =
&AM.getResult<FunctionAnalysisManagerCGSCCProxy>(SCC, LCG).getManager();
}
///}

Expand Down
90 changes: 52 additions & 38 deletions llvm/lib/Analysis/CGSCCPassManager.cpp
View file
Expand Up @@ -68,6 +68,10 @@ PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,
// a pointer that we can update.
LazyCallGraph::SCC *C = &InitialC;

// Get Function analysis manager from its proxy.
FunctionAnalysisManager &FAM =
AM.getCachedResult<FunctionAnalysisManagerCGSCCProxy>(*C)->getManager();

for (auto &Pass : Passes) {
if (DebugLogging)
dbgs() << "Running pass: " << Pass->name() << " on " << *C << "\n";
Expand All @@ -90,6 +94,12 @@ PassManager<LazyCallGraph::SCC, CGSCCAnalysisManager, LazyCallGraph &,

// Update the SCC if necessary.
C = UR.UpdatedC ? UR.UpdatedC : C;
if (UR.UpdatedC) {
// If C is updated, also create a proxy and update FAM inside the result.
auto *ResultFAMCP =
&AM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, G);
ResultFAMCP->updateFAM(FAM);
}

// If the CGSCC pass wasn't able to provide a valid updated SCC, the
// current SCC may simply need to be skipped if invalid.
Expand Down Expand Up @@ -223,23 +233,22 @@ FunctionAnalysisManagerCGSCCProxy::Result
FunctionAnalysisManagerCGSCCProxy::run(LazyCallGraph::SCC &C,
CGSCCAnalysisManager &AM,
LazyCallGraph &CG) {
// Collect the FunctionAnalysisManager from the Module layer and use that to
// build the proxy result.
//
// This allows us to rely on the FunctionAnalysisMangaerModuleProxy to
// invalidate the function analyses.
auto &MAM = AM.getResult<ModuleAnalysisManagerCGSCCProxy>(C, CG).getManager();
// Note: unconditionally getting checking that the proxy exists may get it at
// this point. There are cases when this is being run unnecessarily, but
// it is cheap and having the assertion in place is more valuable.
auto &MAMProxy = AM.getResult<ModuleAnalysisManagerCGSCCProxy>(C, CG);
Module &M = *C.begin()->getFunction().getParent();
auto *FAMProxy = MAM.getCachedResult<FunctionAnalysisManagerModuleProxy>(M);
assert(FAMProxy && "The CGSCC pass manager requires that the FAM module "
"proxy is run on the module prior to entering the CGSCC "
"walk.");

// Note that we special-case invalidation handling of this proxy in the CGSCC
// analysis manager's Module proxy. This avoids the need to do anything
// special here to recompute all of this if ever the FAM's module proxy goes
// away.
return Result(FAMProxy->getManager());
bool ProxyExists =
MAMProxy.cachedResultExists<FunctionAnalysisManagerModuleProxy>(M);
assert(ProxyExists &&
"The CGSCC pass manager requires that the FAM module proxy is run "
"on the module prior to entering the CGSCC walk");
(void)ProxyExists;

// We just return an empty result. The caller will use the updateFAM interface
// to correctly register the relevant FunctionAnalysisManager based on the
// context in which this proxy is run.
return Result();
}

bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(
Expand All @@ -249,8 +258,8 @@ bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(
if (PA.areAllPreserved())
return false; // This is still a valid proxy.

// If this proxy isn't marked as preserved, then even if the result remains
// valid, the key itself may no longer be valid, so we clear everything.
// All updates to preserve valid results are done below, so we don't need to
// invalidate this proxy.
//
// Note that in order to preserve this proxy, a module pass must ensure that
// the FAM has been completely updated to handle the deletion of functions.
Expand All @@ -262,7 +271,7 @@ bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(
for (LazyCallGraph::Node &N : C)
FAM->clear(N.getFunction(), N.getFunction().getName());

return true;
return false;
}

// Directly check if the relevant set is preserved.
Expand Down Expand Up @@ -311,9 +320,10 @@ bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(

} // end namespace llvm

/// When a new SCC is created for the graph and there might be function
/// analysis results cached for the functions now in that SCC two forms of
/// updates are required.
/// When a new SCC is created for the graph we first update the
/// FunctionAnalysisManager in the Proxy's result.
/// As there might be function analysis results cached for the functions now in
/// that SCC, two forms of updates are required.
///
/// First, a proxy from the SCC to the FunctionAnalysisManager needs to be
/// created so that any subsequent invalidation events to the SCC are
Expand All @@ -325,10 +335,9 @@ bool FunctionAnalysisManagerCGSCCProxy::Result::invalidate(
/// function analyses so that they don't retain stale handles.
static void updateNewSCCFunctionAnalyses(LazyCallGraph::SCC &C,
LazyCallGraph &G,
CGSCCAnalysisManager &AM) {
// Get the relevant function analysis manager.
auto &FAM =
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, G).getManager();
CGSCCAnalysisManager &AM,
FunctionAnalysisManager &FAM) {
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, G).updateFAM(FAM);

// Now walk the functions in this SCC and invalidate any function analysis
// results that might have outer dependencies on an SCC analysis.
Expand Down Expand Up @@ -392,8 +401,10 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G,

// If we had a cached FAM proxy originally, we will want to create more of
// them for each SCC that was split off.
bool NeedFAMProxy =
AM.getCachedResult<FunctionAnalysisManagerCGSCCProxy>(*OldC) != nullptr;
FunctionAnalysisManager *FAM = nullptr;
if (auto *FAMProxy =
AM.getCachedResult<FunctionAnalysisManagerCGSCCProxy>(*OldC))
FAM = &FAMProxy->getManager();

// We need to propagate an invalidation call to all but the newly current SCC
// because the outer pass manager won't do that for us after splitting them.
Expand All @@ -407,8 +418,8 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G,
AM.invalidate(*OldC, PA);

// Ensure the now-current SCC's function analyses are updated.
if (NeedFAMProxy)
updateNewSCCFunctionAnalyses(*C, G, AM);
if (FAM)
updateNewSCCFunctionAnalyses(*C, G, AM, *FAM);

for (SCC &NewC : llvm::reverse(make_range(std::next(NewSCCRange.begin()),
NewSCCRange.end()))) {
Expand All @@ -418,8 +429,8 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G,
LLVM_DEBUG(dbgs() << "Enqueuing a newly formed SCC:" << NewC << "\n");

// Ensure new SCCs' function analyses are updated.
if (NeedFAMProxy)
updateNewSCCFunctionAnalyses(NewC, G, AM);
if (FAM)
updateNewSCCFunctionAnalyses(NewC, G, AM, *FAM);

// Also propagate a normal invalidation to the new SCC as only the current
// will get one from the pass manager infrastructure.
Expand All @@ -430,7 +441,8 @@ incorporateNewSCCRange(const SCCRangeT &NewSCCRange, LazyCallGraph &G,

static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
LazyCallGraph &G, LazyCallGraph::SCC &InitialC, LazyCallGraph::Node &N,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR, bool FunctionPass) {
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
FunctionAnalysisManager &FAM, bool FunctionPass) {
using Node = LazyCallGraph::Node;
using Edge = LazyCallGraph::Edge;
using SCC = LazyCallGraph::SCC;
Expand Down Expand Up @@ -686,7 +698,7 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(
// analysis manager, we need to create a proxy in the new current SCC as
// the invalidated SCCs had their functions moved.
if (HasFunctionAnalysisProxy)
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, G);
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(*C, G).updateFAM(FAM);

// Any analyses cached for this SCC are no longer precise as the shape
// has changed by introducing this cycle. However, we have taken care to
Expand Down Expand Up @@ -738,13 +750,15 @@ static LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(

LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForFunctionPass(
LazyCallGraph &G, LazyCallGraph::SCC &InitialC, LazyCallGraph::Node &N,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR) {
return updateCGAndAnalysisManagerForPass(G, InitialC, N, AM, UR,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
FunctionAnalysisManager &FAM) {
return updateCGAndAnalysisManagerForPass(G, InitialC, N, AM, UR, FAM,
/* FunctionPass */ true);
}
LazyCallGraph::SCC &llvm::updateCGAndAnalysisManagerForCGSCCPass(
LazyCallGraph &G, LazyCallGraph::SCC &InitialC, LazyCallGraph::Node &N,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR) {
return updateCGAndAnalysisManagerForPass(G, InitialC, N, AM, UR,
CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
FunctionAnalysisManager &FAM) {
return updateCGAndAnalysisManagerForPass(G, InitialC, N, AM, UR, FAM,
/* FunctionPass */ false);
}

0 comments on commit bd541b2

Please sign in to comment.