[llvm] 04d2019 - Revert "[StandardInstrumentations] Check function analysis invalidation in module passes as well"
Arthur Eubanks via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 15 13:27:55 PDT 2023
Author: Arthur Eubanks
Date: 2023-03-15T13:27:36-07:00
New Revision: 04d20195d6b3747a3cdc882105320b32cb192b8a
URL: https://github.com/llvm/llvm-project/commit/04d20195d6b3747a3cdc882105320b32cb192b8a
DIFF: https://github.com/llvm/llvm-project/commit/04d20195d6b3747a3cdc882105320b32cb192b8a.diff
LOG: Revert "[StandardInstrumentations] Check function analysis invalidation in module passes as well"
This reverts commit d6c0724eb158efcdcd4e31289dcb954a441c4939.
Breaks clang/flang builds.
Added:
Modified:
llvm/include/llvm/Passes/StandardInstrumentations.h
llvm/lib/LTO/LTOBackend.cpp
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
llvm/lib/Passes/PassBuilderBindings.cpp
llvm/lib/Passes/StandardInstrumentations.cpp
llvm/tools/opt/NewPMDriver.cpp
llvm/unittests/IR/PassManagerTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Passes/StandardInstrumentations.h b/llvm/include/llvm/Passes/StandardInstrumentations.h
index 759b79dc2b0ba..3b748f5e9f44c 100644
--- a/llvm/include/llvm/Passes/StandardInstrumentations.h
+++ b/llvm/include/llvm/Passes/StandardInstrumentations.h
@@ -153,7 +153,7 @@ class PreservedCFGCheckerInstrumentation {
#endif
void registerCallbacks(PassInstrumentationCallbacks &PIC,
- ModuleAnalysisManager &MAM);
+ FunctionAnalysisManager &FAM);
};
// Base class for classes that report changes to the IR.
@@ -574,7 +574,7 @@ class StandardInstrumentations {
// Register all the standard instrumentation callbacks. If \p FAM is nullptr
// then PreservedCFGChecker is not enabled.
void registerCallbacks(PassInstrumentationCallbacks &PIC,
- ModuleAnalysisManager *MAM = nullptr);
+ FunctionAnalysisManager *FAM = nullptr);
TimePassesHandler &getTimePasses() { return TimePasses; }
};
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index d574ae5737d55..4c41a382276a4 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -260,7 +260,7 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(Mod.getContext(), Conf.DebugPassManager);
- SI.registerCallbacks(PIC, &MAM);
+ SI.registerCallbacks(PIC, &FAM);
PassBuilder PB(TM, Conf.PTO, PGOOpt, &PIC);
RegisterPassPlugins(Conf.PassPlugins, PB);
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 595906a44bb44..5b137a8f8cb34 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -245,7 +245,7 @@ static void optimizeModule(Module &TheModule, TargetMachine &TM,
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(TheModule.getContext(), DebugPassManager);
- SI.registerCallbacks(PIC, &MAM);
+ SI.registerCallbacks(PIC, &FAM);
PipelineTuningOptions PTO;
PTO.LoopVectorization = true;
PTO.SLPVectorization = true;
diff --git a/llvm/lib/Passes/PassBuilderBindings.cpp b/llvm/lib/Passes/PassBuilderBindings.cpp
index 2a49ae6e30e0a..a87c0e6dc0a37 100644
--- a/llvm/lib/Passes/PassBuilderBindings.cpp
+++ b/llvm/lib/Passes/PassBuilderBindings.cpp
@@ -66,7 +66,7 @@ LLVMErrorRef LLVMRunPasses(LLVMModuleRef M, const char *Passes,
PB.crossRegisterProxies(LAM, FAM, CGAM, MAM);
StandardInstrumentations SI(Mod->getContext(), Debug, VerifyEach);
- SI.registerCallbacks(PIC, &MAM);
+ SI.registerCallbacks(PIC, &FAM);
ModulePassManager MPM;
if (VerifyEach) {
MPM.addPass(VerifierPass());
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index af9751745861d..8008986046188 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -1075,46 +1075,29 @@ bool PreservedCFGCheckerInstrumentation::CFG::invalidate(
PAC.preservedSet<CFGAnalyses>());
}
-static SmallVector<Function *, 1> GetFunctions(Any IR) {
- SmallVector<Function *, 1> Functions;
-
- if (const auto **MaybeF = any_cast<const Function *>(&IR)) {
- Functions.push_back(*const_cast<Function **>(MaybeF));
- } else if (const auto **MaybeM = any_cast<const Module *>(&IR)) {
- for (Function &F : **const_cast<Module **>(MaybeM))
- Functions.push_back(&F);
- }
- return Functions;
-}
-
void PreservedCFGCheckerInstrumentation::registerCallbacks(
- PassInstrumentationCallbacks &PIC, ModuleAnalysisManager &MAM) {
+ PassInstrumentationCallbacks &PIC, FunctionAnalysisManager &FAM) {
if (!VerifyAnalysisInvalidation)
return;
- bool Registered = false;
- PIC.registerBeforeNonSkippedPassCallback([this, &MAM, Registered](
- StringRef P, Any IR) mutable {
+ FAM.registerPass([&] { return PreservedCFGCheckerAnalysis(); });
+ FAM.registerPass([&] { return PreservedFunctionHashAnalysis(); });
+
+ PIC.registerBeforeNonSkippedPassCallback(
+ [this, &FAM](StringRef P, Any IR) {
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
- assert(&PassStack.emplace_back(P));
+ assert(&PassStack.emplace_back(P));
#endif
- (void)this;
-
- auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(
- *const_cast<Module *>(unwrapModule(IR, /*Force=*/true)))
- .getManager();
- if (!Registered) {
- FAM.registerPass([&] { return PreservedCFGCheckerAnalysis(); });
- FAM.registerPass([&] { return PreservedFunctionHashAnalysis(); });
- Registered = true;
- }
+ (void)this;
+ const auto **F = any_cast<const Function *>(&IR);
+ if (!F)
+ return;
- for (Function *F : GetFunctions(IR)) {
- // Make sure a fresh CFG snapshot is available before the pass.
- FAM.getResult<PreservedCFGCheckerAnalysis>(*F);
- FAM.getResult<PreservedFunctionHashAnalysis>(*F);
- }
- });
+ // Make sure a fresh CFG snapshot is available before the pass.
+ FAM.getResult<PreservedCFGCheckerAnalysis>(*const_cast<Function *>(*F));
+ FAM.getResult<PreservedFunctionHashAnalysis>(
+ *const_cast<Function *>(*F));
+ });
PIC.registerAfterPassInvalidatedCallback(
[this](StringRef P, const PreservedAnalyses &PassPA) {
@@ -1125,7 +1108,7 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(
(void)this;
});
- PIC.registerAfterPassCallback([this, &MAM](StringRef P, Any IR,
+ PIC.registerAfterPassCallback([this, &FAM](StringRef P, Any IR,
const PreservedAnalyses &PassPA) {
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(PassStack.pop_back_val() == P &&
@@ -1133,42 +1116,36 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(
#endif
(void)this;
- // We have to get the FAM via the MAM, rather than directly use a passed in
- // FAM because if MAM has not cached the FAM, it won't invalidate function
- // analyses in FAM.
- auto &FAM = MAM.getResult<FunctionAnalysisManagerModuleProxy>(
- *const_cast<Module *>(unwrapModule(IR, /*Force=*/true)))
- .getManager();
-
- for (Function *F : GetFunctions(IR)) {
- if (auto *HashBefore =
- FAM.getCachedResult<PreservedFunctionHashAnalysis>(*F)) {
- if (HashBefore->Hash != StructuralHash(*F)) {
- report_fatal_error(formatv(
- "Function @{0} changed by {1} without invalidating analyses",
- F->getName(), P));
- }
+ const auto **MaybeF = any_cast<const Function *>(&IR);
+ if (!MaybeF)
+ return;
+ Function &F = *const_cast<Function *>(*MaybeF);
+
+ if (auto *HashBefore =
+ FAM.getCachedResult<PreservedFunctionHashAnalysis>(F)) {
+ if (HashBefore->Hash != StructuralHash(F)) {
+ report_fatal_error(formatv(
+ "Function @{0} changed by {1} without invalidating analyses",
+ F.getName(), P));
}
-
- auto CheckCFG = [](StringRef Pass, StringRef FuncName,
- const CFG &GraphBefore, const CFG &GraphAfter) {
- if (GraphAfter == GraphBefore)
- return;
-
- dbgs()
- << "Error: " << Pass
- << " does not invalidate CFG analyses but CFG changes detected in "
- "function @"
- << FuncName << ":\n";
- CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
- report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
- };
-
- if (auto *GraphBefore =
- FAM.getCachedResult<PreservedCFGCheckerAnalysis>(*F))
- CheckCFG(P, F->getName(), *GraphBefore,
- CFG(F, /* TrackBBLifetime */ false));
}
+
+ auto CheckCFG = [](StringRef Pass, StringRef FuncName,
+ const CFG &GraphBefore, const CFG &GraphAfter) {
+ if (GraphAfter == GraphBefore)
+ return;
+
+ dbgs() << "Error: " << Pass
+ << " does not invalidate CFG analyses but CFG changes detected in "
+ "function @"
+ << FuncName << ":\n";
+ CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+ report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+ };
+
+ if (auto *GraphBefore = FAM.getCachedResult<PreservedCFGCheckerAnalysis>(F))
+ CheckCFG(P, F.getName(), *GraphBefore,
+ CFG(&F, /* TrackBBLifetime */ false));
});
}
@@ -2198,7 +2175,7 @@ void PrintCrashIRInstrumentation::registerCallbacks(
}
void StandardInstrumentations::registerCallbacks(
- PassInstrumentationCallbacks &PIC, ModuleAnalysisManager *MAM) {
+ PassInstrumentationCallbacks &PIC, FunctionAnalysisManager *FAM) {
PrintIR.registerCallbacks(PIC);
PrintPass.registerCallbacks(PIC);
TimePasses.registerCallbacks(PIC);
@@ -2212,8 +2189,8 @@ void StandardInstrumentations::registerCallbacks(
WebsiteChangeReporter.registerCallbacks(PIC);
ChangeTester.registerCallbacks(PIC);
PrintCrashIR.registerCallbacks(PIC);
- if (MAM)
- PreservedCFGChecker.registerCallbacks(PIC, *MAM);
+ if (FAM)
+ PreservedCFGChecker.registerCallbacks(PIC, *FAM);
// TimeProfiling records the pass running time cost.
// Its 'BeforePassCallback' can be appended at the tail of all the
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index bcf6a7f3f9aa8..697f2649d20b0 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -395,7 +395,7 @@ bool llvm::runPassPipeline(StringRef Arg0, Module &M, TargetMachine *TM,
PrintPassOpts.SkipAnalyses = DebugPM == DebugLogging::Quiet;
StandardInstrumentations SI(M.getContext(), DebugPM != DebugLogging::None,
VerifyEachPass, PrintPassOpts);
- SI.registerCallbacks(PIC, &MAM);
+ SI.registerCallbacks(PIC, &FAM);
DebugifyEachInstrumentation Debugify;
DebugifyStatsMap DIStatsMap;
DebugInfoPerPass DebugInfoBeforePass;
diff --git a/llvm/unittests/IR/PassManagerTest.cpp b/llvm/unittests/IR/PassManagerTest.cpp
index dda1d0c7bbd80..4c8be3702bf0f 100644
--- a/llvm/unittests/IR/PassManagerTest.cpp
+++ b/llvm/unittests/IR/PassManagerTest.cpp
@@ -824,13 +824,10 @@ TEST_F(PassManagerTest, FunctionPassCFGChecker) {
auto *F = M->getFunction("foo");
FunctionAnalysisManager FAM;
- ModuleAnalysisManager MAM;
FunctionPassManager FPM;
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
- SI.registerCallbacks(PIC, &MAM);
- MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
- MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
+ SI.registerCallbacks(PIC, &FAM);
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -873,13 +870,10 @@ TEST_F(PassManagerTest, FunctionPassCFGCheckerInvalidateAnalysis) {
auto *F = M->getFunction("foo");
FunctionAnalysisManager FAM;
- ModuleAnalysisManager MAM;
FunctionPassManager FPM;
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
- SI.registerCallbacks(PIC, &MAM);
- MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
- MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
+ SI.registerCallbacks(PIC, &FAM);
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -941,13 +935,10 @@ TEST_F(PassManagerTest, FunctionPassCFGCheckerWrapped) {
auto *F = M->getFunction("foo");
FunctionAnalysisManager FAM;
- ModuleAnalysisManager MAM;
FunctionPassManager FPM;
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ true);
- SI.registerCallbacks(PIC, &MAM);
- MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
- MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
+ SI.registerCallbacks(PIC, &FAM);
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@@ -970,7 +961,7 @@ struct WrongFunctionPass : PassInfoMixin<WrongFunctionPass> {
static StringRef name() { return "WrongFunctionPass"; }
};
-TEST_F(PassManagerTest, FunctionPassMissedFunctionAnalysisInvalidation) {
+TEST_F(PassManagerTest, FunctionAnalysisMissedInvalidation) {
LLVMContext Context;
auto M = parseIR(Context, "define void @foo() {\n"
" %a = add i32 0, 0\n"
@@ -978,12 +969,9 @@ TEST_F(PassManagerTest, FunctionPassMissedFunctionAnalysisInvalidation) {
"}\n");
FunctionAnalysisManager FAM;
- ModuleAnalysisManager MAM;
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ false);
- SI.registerCallbacks(PIC, &MAM);
- MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
- MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
+ SI.registerCallbacks(PIC, &FAM);
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FunctionPassManager FPM;
@@ -993,39 +981,6 @@ TEST_F(PassManagerTest, FunctionPassMissedFunctionAnalysisInvalidation) {
EXPECT_DEATH(FPM.run(*F, FAM), "Function @foo changed by WrongFunctionPass without invalidating analyses");
}
-struct WrongModulePass : PassInfoMixin<WrongModulePass> {
- PreservedAnalyses run(Module &M, ModuleAnalysisManager &MAM) {
- for (Function &F : M)
- F.getEntryBlock().begin()->eraseFromParent();
-
- return PreservedAnalyses::all();
- }
- static StringRef name() { return "WrongModulePass"; }
-};
-
-TEST_F(PassManagerTest, ModulePassMissedFunctionAnalysisInvalidation) {
- LLVMContext Context;
- auto M = parseIR(Context, "define void @foo() {\n"
- " %a = add i32 0, 0\n"
- " ret void\n"
- "}\n");
-
- FunctionAnalysisManager FAM;
- ModuleAnalysisManager MAM;
- PassInstrumentationCallbacks PIC;
- StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ false);
- SI.registerCallbacks(PIC, &MAM);
- MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
- MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
- FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
-
- ModulePassManager MPM;
- MPM.addPass(WrongModulePass());
-
- EXPECT_DEATH(
- MPM.run(*M, MAM),
- "Function @foo changed by WrongModulePass without invalidating analyses");
-}
-
#endif
+
}
More information about the llvm-commits
mailing list