From bfc6590e66beca9d6dc7fe613de9b49b0bda6d5b Mon Sep 17 00:00:00 2001 From: Arthur Eubanks Date: Tue, 14 Mar 2023 14:25:57 -0700 Subject: [PATCH] [PassManager] Run PassInstrumentation after analysis invalidation This allows instrumentation to inspect cached analyses to verify them. The CGSCC PassInstrumentation previously ran `runAfterPass()` on the original SCC, but really it should be running on UpdatedC when relevant since that's the relevant SCC after the pass. Reviewed By: nikic Differential Revision: https://reviews.llvm.org/D146096 --- llvm/include/llvm/IR/PassManager.h | 8 ++--- llvm/lib/Analysis/CGSCCPassManager.cpp | 45 +++++++++++++--------------- llvm/lib/CodeGen/MachinePassManager.cpp | 2 +- llvm/lib/IR/PassManager.cpp | 3 +- llvm/lib/Passes/StandardInstrumentations.cpp | 4 --- llvm/test/Other/scc-deleted-printer.ll | 2 +- 6 files changed, 28 insertions(+), 36 deletions(-) diff --git a/llvm/include/llvm/IR/PassManager.h b/llvm/include/llvm/IR/PassManager.h index a9d54bd..5fe28f8 100644 --- a/llvm/include/llvm/IR/PassManager.h +++ b/llvm/include/llvm/IR/PassManager.h @@ -516,14 +516,14 @@ public: PreservedAnalyses PassPA = Pass->run(IR, AM, ExtraArgs...); - // Call onto PassInstrumentation's AfterPass callbacks immediately after - // running the pass. - PI.runAfterPass(*Pass, IR, PassPA); - // Update the analysis manager as each pass runs and potentially // invalidates analyses. AM.invalidate(IR, PassPA); + // Call onto PassInstrumentation's AfterPass callbacks immediately after + // running the pass. + PI.runAfterPass(*Pass, IR, PassPA); + // Finally, intersect the preserved analyses to compute the aggregate // preserved set for this pass manager. PA.intersect(std::move(PassPA)); diff --git a/llvm/lib/Analysis/CGSCCPassManager.cpp b/llvm/lib/Analysis/CGSCCPassManager.cpp index 6c2db5b..facb9c8 100644 --- a/llvm/lib/Analysis/CGSCCPassManager.cpp +++ b/llvm/lib/Analysis/CGSCCPassManager.cpp @@ -86,11 +86,6 @@ PassManagerrun(*C, AM, G, UR); - if (UR.InvalidatedSCCs.count(C)) - PI.runAfterPassInvalidated(*Pass, PassPA); - else - PI.runAfterPass(*Pass, *C, PassPA); - // Update the SCC if necessary. C = UR.UpdatedC ? UR.UpdatedC : C; if (UR.UpdatedC) { @@ -107,6 +102,7 @@ PassManager(*Pass, PassPA); LLVM_DEBUG(dbgs() << "Skipping invalidated root or island SCC!\n"); break; } @@ -117,6 +113,8 @@ PassManager(*Pass, *C, PassPA); } // Before we mark all of *this* SCC's analyses as preserved below, intersect @@ -276,11 +274,6 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) { PreservedAnalyses PassPA = Pass->run(*C, CGAM, CG, UR); - if (UR.InvalidatedSCCs.count(C)) - PI.runAfterPassInvalidated(*Pass, PassPA); - else - PI.runAfterPass(*Pass, *C, PassPA); - // Update the SCC and RefSCC if necessary. C = UR.UpdatedC ? UR.UpdatedC : C; @@ -301,6 +294,7 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) { // 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)) { + PI.runAfterPassInvalidated(*Pass, PassPA); LLVM_DEBUG(dbgs() << "Skipping invalidated root or island SCC!\n"); break; } @@ -316,6 +310,8 @@ ModuleToPostOrderCGSCCPassAdaptor::run(Module &M, ModuleAnalysisManager &AM) { // processed. CGAM.invalidate(*C, PassPA); + PI.runAfterPass(*Pass, *C, PassPA); + // The pass may have restructured the call graph and refined the // current SCC and/or RefSCC. We need to update our current SCC and // RefSCC pointers to follow these. Also, when the current SCC is @@ -408,25 +404,27 @@ PreservedAnalyses DevirtSCCRepeatedPass::run(LazyCallGraph::SCC &InitialC, PreservedAnalyses PassPA = Pass->run(*C, AM, CG, UR); - if (UR.InvalidatedSCCs.count(C)) - PI.runAfterPassInvalidated(*Pass, PassPA); - else - PI.runAfterPass(*Pass, *C, PassPA); - PA.intersect(PassPA); - // If the SCC structure has changed, bail immediately and let the outer - // CGSCC layer handle any iteration to reflect the refined structure. - if (UR.UpdatedC && UR.UpdatedC != C) - break; - // 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)) { + PI.runAfterPassInvalidated(*Pass, PassPA); LLVM_DEBUG(dbgs() << "Skipping invalidated root or island SCC!\n"); break; } + // 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); + + PI.runAfterPass(*Pass, *C, PassPA); + + // If the SCC structure has changed, bail immediately and let the outer + // CGSCC layer handle any iteration to reflect the refined structure. + if (UR.UpdatedC && UR.UpdatedC != C) + break; + assert(C->begin() != C->end() && "Cannot have an empty SCC!"); // Check whether any of the handles were devirtualized. @@ -490,10 +488,6 @@ PreservedAnalyses DevirtSCCRepeatedPass::run(LazyCallGraph::SCC &InitialC, // Move over the new call counts in preparation for iterating. CallCounts = std::move(NewCallCounts); - - // 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); } // Note that we don't add any preserved entries here unlike a more normal @@ -539,13 +533,14 @@ PreservedAnalyses CGSCCToFunctionPassAdaptor::run(LazyCallGraph::SCC &C, continue; PreservedAnalyses PassPA = Pass->run(F, FAM); - PI.runAfterPass(*Pass, F, PassPA); // We know that the function pass couldn't have invalidated any other // function's analyses (that's the contract of a function pass), so // directly handle the function analysis manager's invalidation here. FAM.invalidate(F, EagerlyInvalidate ? PreservedAnalyses::none() : PassPA); + PI.runAfterPass(*Pass, F, PassPA); + // Then intersect the preserved set so that invalidation of module // analyses will eventually occur when the module pass completes. PA.intersect(std::move(PassPA)); diff --git a/llvm/lib/CodeGen/MachinePassManager.cpp b/llvm/lib/CodeGen/MachinePassManager.cpp index 039634f..439ff8b 100644 --- a/llvm/lib/CodeGen/MachinePassManager.cpp +++ b/llvm/lib/CodeGen/MachinePassManager.cpp @@ -91,8 +91,8 @@ Error MachineFunctionPassManager::run(Module &M, // TODO: EmitSizeRemarks PreservedAnalyses PassPA = P->run(MF, MFAM); - PI.runAfterPass(*P, MF, PassPA); MFAM.invalidate(MF, PassPA); + PI.runAfterPass(*P, MF, PassPA); } } } while (true); diff --git a/llvm/lib/IR/PassManager.cpp b/llvm/lib/IR/PassManager.cpp index de46749..92b729c 100644 --- a/llvm/lib/IR/PassManager.cpp +++ b/llvm/lib/IR/PassManager.cpp @@ -122,13 +122,14 @@ PreservedAnalyses ModuleToFunctionPassAdaptor::run(Module &M, continue; PreservedAnalyses PassPA = Pass->run(F, FAM); - PI.runAfterPass(*Pass, F, PassPA); // We know that the function pass couldn't have invalidated any other // function's analyses (that's the contract of a function pass), so // directly handle the function analysis manager's invalidation here. FAM.invalidate(F, EagerlyInvalidate ? PreservedAnalyses::none() : PassPA); + PI.runAfterPass(*Pass, F, PassPA); + // Then intersect the preserved set so that invalidation of module // analyses will eventually occur when the module pass completes. PA.intersect(std::move(PassPA)); diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp index 3a78260..761da252 100644 --- a/llvm/lib/Passes/StandardInstrumentations.cpp +++ b/llvm/lib/Passes/StandardInstrumentations.cpp @@ -1098,10 +1098,6 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks( if (!F) return; - if (!PassPA.allAnalysesInSetPreserved() && - !PassPA.allAnalysesInSetPreserved>()) - return; - auto CheckCFG = [](StringRef Pass, StringRef FuncName, const CFG &GraphBefore, const CFG &GraphAfter) { if (GraphAfter == GraphBefore) diff --git a/llvm/test/Other/scc-deleted-printer.ll b/llvm/test/Other/scc-deleted-printer.ll index 86171d9..c7455ff 100644 --- a/llvm/test/Other/scc-deleted-printer.ll +++ b/llvm/test/Other/scc-deleted-printer.ll @@ -4,7 +4,7 @@ ; RUN: -passes=inline -print-before-all -print-after-all -print-module-scope | FileCheck %s ; CHECK: IR Dump Before InlinerPass on (tester, foo) -; CHECK: IR Dump After InlinerPass on (tester, foo) (invalidated) +; CHECK: IR Dump After InlinerPass on (tester, foo) ; CHECK: IR Dump Before InlinerPass on (tester) ; CHECK: IR Dump After InlinerPass on (tester) -- 2.7.4