shylie/llvm-project
1
0
Fork 0
Code Issues Pull Requests Actions 6 Packages Projects Releases Wiki Activity

Reland [StandardInstrumentations] Check function analysis invalidation in module passes as well

Browse Source 
See comments for why we now need to pass in the MAM instead of the FAM.

Reviewed By: nikic

Differential Revision: https://reviews.llvm.org/D146160
This commit is contained in:
Arthur Eubanks 2023-03-15 11:46:44 -07:00
parent 04d20195d6
commit 6a6994cc9b
9 changed files with 129 additions and 61 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
clang/lib/CodeGen/BackendUtil.cpp
View File

@ -837,7 +837,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
TheModule->getContext(),
(CodeGenOpts.DebugPassManager || DebugPassStructure),
/*VerifyEach*/ false, PrintPassOpts);
SI.registerCallbacks(PIC, &FAM);
SI.registerCallbacks(PIC, &MAM);
PassBuilder PB(TM.get(), PTO, PGOOpt, &PIC);
if (CodeGenOpts.EnableAssignmentTracking) {

2
flang/lib/Frontend/FrontendActions.cpp
View File

@ -699,7 +699,7 @@ void CodeGenAction::runOptimizationPipeline(llvm::raw_pwrite_stream &os) {
std::optional<llvm::PGOOptions> pgoOpt;
llvm::StandardInstrumentations si(llvmModule->getContext(),
opts.DebugPassManager);
si.registerCallbacks(pic, &fam);
si.registerCallbacks(pic, &mam);
llvm::PassBuilder pb(tm.get(), pto, pgoOpt, &pic);
// Attempt to load pass plugins and register their callbacks with PB.

4
llvm/include/llvm/Passes/StandardInstrumentations.h
View File

@ -153,7 +153,7 @@ public:
#endif
void registerCallbacks(PassInstrumentationCallbacks &PIC,
FunctionAnalysisManager &FAM);
ModuleAnalysisManager &MAM);
};
// Base class for classes that report changes to the IR.
@ -574,7 +574,7 @@ public:
// Register all the standard instrumentation callbacks. If \p FAM is nullptr
// then PreservedCFGChecker is not enabled.
void registerCallbacks(PassInstrumentationCallbacks &PIC,
FunctionAnalysisManager *FAM = nullptr);
ModuleAnalysisManager *MAM = nullptr);
TimePassesHandler &getTimePasses() { return TimePasses; }
};

2
llvm/lib/LTO/LTOBackend.cpp
View File

@ -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, &FAM);
SI.registerCallbacks(PIC, &MAM);
PassBuilder PB(TM, Conf.PTO, PGOOpt, &PIC);
RegisterPassPlugins(Conf.PassPlugins, PB);

2
llvm/lib/LTO/ThinLTOCodeGenerator.cpp
View File

@ -245,7 +245,7 @@ static void optimizeModule(Module &TheModule, TargetMachine &TM,
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(TheModule.getContext(), DebugPassManager);
SI.registerCallbacks(PIC, &FAM);
SI.registerCallbacks(PIC, &MAM);
PipelineTuningOptions PTO;
PTO.LoopVectorization = true;
PTO.SLPVectorization = true;

2
llvm/lib/Passes/PassBuilderBindings.cpp
View File

@ -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, &FAM);
SI.registerCallbacks(PIC, &MAM);
ModulePassManager MPM;
if (VerifyEach) {
MPM.addPass(VerifierPass());

117
llvm/lib/Passes/StandardInstrumentations.cpp
View File

@ -1075,29 +1075,46 @@ 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, FunctionAnalysisManager &FAM) {
PassInstrumentationCallbacks &PIC, ModuleAnalysisManager &MAM) {
if (!VerifyAnalysisInvalidation)
return;
FAM.registerPass([&] { return PreservedCFGCheckerAnalysis(); });
FAM.registerPass([&] { return PreservedFunctionHashAnalysis(); });
PIC.registerBeforeNonSkippedPassCallback(
[this, &FAM](StringRef P, Any IR) {
bool Registered = false;
PIC.registerBeforeNonSkippedPassCallback([this, &MAM, Registered](
StringRef P, Any IR) mutable {
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(&PassStack.emplace_back(P));
assert(&PassStack.emplace_back(P));
#endif
(void)this;
const auto **F = any_cast<const Function *>(&IR);
if (!F)
return;
(void)this;
// 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));
});
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;
}
for (Function *F : GetFunctions(IR)) {
// Make sure a fresh CFG snapshot is available before the pass.
FAM.getResult<PreservedCFGCheckerAnalysis>(*F);
FAM.getResult<PreservedFunctionHashAnalysis>(*F);
}
});
PIC.registerAfterPassInvalidatedCallback(
[this](StringRef P, const PreservedAnalyses &PassPA) {
@ -1108,7 +1125,7 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(
(void)this;
});
PIC.registerAfterPassCallback([this, &FAM](StringRef P, Any IR,
PIC.registerAfterPassCallback([this, &MAM](StringRef P, Any IR,
const PreservedAnalyses &PassPA) {
#ifdef LLVM_ENABLE_ABI_BREAKING_CHECKS
assert(PassStack.pop_back_val() == P &&
@ -1116,36 +1133,42 @@ void PreservedCFGCheckerInstrumentation::registerCallbacks(
#endif
(void)this;
const auto **MaybeF = any_cast<const Function *>(&IR);
if (!MaybeF)
return;
Function &F = *const_cast<Function *>(*MaybeF);
// 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();
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));
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));
}
}
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));
});
}
@ -2175,7 +2198,7 @@ void PrintCrashIRInstrumentation::registerCallbacks(
}
void StandardInstrumentations::registerCallbacks(
PassInstrumentationCallbacks &PIC, FunctionAnalysisManager *FAM) {
PassInstrumentationCallbacks &PIC, ModuleAnalysisManager *MAM) {
PrintIR.registerCallbacks(PIC);
PrintPass.registerCallbacks(PIC);
TimePasses.registerCallbacks(PIC);
@ -2189,8 +2212,8 @@ void StandardInstrumentations::registerCallbacks(
WebsiteChangeReporter.registerCallbacks(PIC);
ChangeTester.registerCallbacks(PIC);
PrintCrashIR.registerCallbacks(PIC);
if (FAM)
PreservedCFGChecker.registerCallbacks(PIC, *FAM);
if (MAM)
PreservedCFGChecker.registerCallbacks(PIC, *MAM);
// TimeProfiling records the pass running time cost.
// Its 'BeforePassCallback' can be appended at the tail of all the

2
llvm/tools/opt/NewPMDriver.cpp
View File

@ -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, &FAM);
SI.registerCallbacks(PIC, &MAM);
DebugifyEachInstrumentation Debugify;
DebugifyStatsMap DIStatsMap;
DebugInfoPerPass DebugInfoBeforePass;

57
llvm/unittests/IR/PassManagerTest.cpp
View File

@ -824,10 +824,13 @@ 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, &FAM);
SI.registerCallbacks(PIC, &MAM);
MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@ -870,10 +873,13 @@ 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, &FAM);
SI.registerCallbacks(PIC, &MAM);
MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@ -935,10 +941,13 @@ 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, &FAM);
SI.registerCallbacks(PIC, &MAM);
MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return DominatorTreeAnalysis(); });
FAM.registerPass([&] { return AssumptionAnalysis(); });
@ -961,7 +970,7 @@ struct WrongFunctionPass : PassInfoMixin<WrongFunctionPass> {
static StringRef name() { return "WrongFunctionPass"; }
};
TEST_F(PassManagerTest, FunctionAnalysisMissedInvalidation) {
TEST_F(PassManagerTest, FunctionPassMissedFunctionAnalysisInvalidation) {
LLVMContext Context;
auto M = parseIR(Context, "define void @foo() {\n"
" %a = add i32 0, 0\n"
@ -969,9 +978,12 @@ TEST_F(PassManagerTest, FunctionAnalysisMissedInvalidation) {
"}\n");
FunctionAnalysisManager FAM;
ModuleAnalysisManager MAM;
PassInstrumentationCallbacks PIC;
StandardInstrumentations SI(M->getContext(), /*DebugLogging*/ false);
SI.registerCallbacks(PIC, &FAM);
SI.registerCallbacks(PIC, &MAM);
MAM.registerPass([&] { return FunctionAnalysisManagerModuleProxy(FAM); });
MAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FAM.registerPass([&] { return PassInstrumentationAnalysis(&PIC); });
FunctionPassManager FPM;
@ -981,6 +993,39 @@ TEST_F(PassManagerTest, FunctionAnalysisMissedInvalidation) {
EXPECT_DEATH(FPM.run(*F, FAM), "Function @foo changed by WrongFunctionPass without invalidating analyses");
}
#endif
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
}