[llvm] [LLVM][Coroutines] Switch CoroAnnotationElidePass to a FunctionPass (PR #107897)
Yuxuan Chen via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 9 11:29:22 PDT 2024
https://github.com/yuxuanchen1997 created https://github.com/llvm/llvm-project/pull/107897
After landing https://github.com/llvm/llvm-project/pull/99285 we found that the call graph update was causing the following crash when expensive checks are turned on
```
opt: /home/ych/llvm-project/llvm/lib/Analysis/CGSCCPassManager.cpp:982: LazyCallGraph::SCC &updateCGAndAnalysisManagerForPass(LazyCallGraph &, LazyCallGraph::SCC &, LazyCallGraph::Node &, CGSCCAnalysisManager &, CGSCCUpdateResult &, FunctionAnalysisManager &, bool): Assertion `(RC == &TargetRC || RC->isAncestorOf(Targe
tRC)) && "New call edge is not trivial!"' failed.
```
This is because the call graph updater doesn't support inserting new edges yet.
After reading the code in `CGSCCToFunctionPassAdaptor`, I am convinced that `CoroAnnotationElidePass` can be a FunctionPass and rely on the adaptor to update the call graph for us, so long as we properly invalidate the caller's analyses.
After this patch, `llvm/test/Transforms/Coroutines/coro-transform-must-elide.ll` no longer fails under expensive checks.
>From 856126b85e2b3a2cacf54190e29b8c7ab0eae4f0 Mon Sep 17 00:00:00 2001
From: Yuxuan Chen <ych at meta.com>
Date: Mon, 9 Sep 2024 11:03:48 -0700
Subject: [PATCH] [LLVM][Coroutines] Switch CoroAnnotationElidePass to a
FunctionPass
---
.../Coroutines/CoroAnnotationElide.h | 10 ++---
llvm/lib/Passes/PassBuilderPipelines.cpp | 6 +--
llvm/lib/Passes/PassRegistry.def | 2 +-
.../Coroutines/CoroAnnotationElide.cpp | 43 +++++++------------
4 files changed, 22 insertions(+), 39 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h b/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
index 352c9e14526697..986a5dbd1ed0fe 100644
--- a/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
+++ b/llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h
@@ -17,18 +17,14 @@
#ifndef LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
#define LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
-#include "llvm/Analysis/CGSCCPassManager.h"
-#include "llvm/Analysis/LazyCallGraph.h"
#include "llvm/IR/PassManager.h"
namespace llvm {
-struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
- CoroAnnotationElidePass() {}
-
- PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
- LazyCallGraph &CG, CGSCCUpdateResult &UR);
+class Function;
+struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
+ PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
static bool isRequired() { return false; }
};
} // end namespace llvm
diff --git a/llvm/lib/Passes/PassBuilderPipelines.cpp b/llvm/lib/Passes/PassBuilderPipelines.cpp
index 4e8e3dcdff4428..b063caa5279afa 100644
--- a/llvm/lib/Passes/PassBuilderPipelines.cpp
+++ b/llvm/lib/Passes/PassBuilderPipelines.cpp
@@ -976,7 +976,8 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));
- MainCGPipeline.addPass(CoroAnnotationElidePass());
+ MainCGPipeline.addPass(
+ createCGSCCToFunctionPassAdaptor(CoroAnnotationElidePass()));
}
// Make sure we don't affect potential future NoRerun CGSCC adaptors.
@@ -1022,8 +1023,7 @@ PassBuilder::buildModuleInlinerPipeline(OptimizationLevel Level,
if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(
CoroSplitPass(Level != OptimizationLevel::O0)));
- MPM.addPass(
- createModuleToPostOrderCGSCCPassAdaptor(CoroAnnotationElidePass()));
+ MPM.addPass(createModuleToFunctionPassAdaptor(CoroAnnotationElidePass()));
}
return MPM;
diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def
index 4f5f680a6e9535..3dc7f185f330c5 100644
--- a/llvm/lib/Passes/PassRegistry.def
+++ b/llvm/lib/Passes/PassRegistry.def
@@ -243,7 +243,6 @@ CGSCC_PASS("attributor-light-cgscc", AttributorLightCGSCCPass())
CGSCC_PASS("invalidate<all>", InvalidateAllAnalysesPass())
CGSCC_PASS("no-op-cgscc", NoOpCGSCCPass())
CGSCC_PASS("openmp-opt-cgscc", OpenMPOptCGSCCPass())
-CGSCC_PASS("coro-annotation-elide", CoroAnnotationElidePass())
#undef CGSCC_PASS
#ifndef CGSCC_PASS_WITH_PARAMS
@@ -343,6 +342,7 @@ FUNCTION_PASS("codegenprepare", CodeGenPreparePass(TM))
FUNCTION_PASS("consthoist", ConstantHoistingPass())
FUNCTION_PASS("constraint-elimination", ConstraintEliminationPass())
FUNCTION_PASS("coro-elide", CoroElidePass())
+FUNCTION_PASS("coro-annotation-elide", CoroAnnotationElidePass())
FUNCTION_PASS("correlated-propagation", CorrelatedValuePropagationPass())
FUNCTION_PASS("count-visits", CountVisitsPass())
FUNCTION_PASS("dce", DCEPass())
diff --git a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
index 27a370a5d8fbf9..c3f258eb083dae 100644
--- a/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp
@@ -16,7 +16,6 @@
#include "llvm/Transforms/Coroutines/CoroAnnotationElide.h"
-#include "llvm/Analysis/CGSCCPassManager.h"
#include "llvm/Analysis/LazyCallGraph.h"
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
#include "llvm/IR/Analysis.h"
@@ -96,24 +95,15 @@ static void processCall(CallBase *CB, Function *Caller, Function *NewCallee,
CB->eraseFromParent();
}
-PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
- CGSCCAnalysisManager &AM,
- LazyCallGraph &CG,
- CGSCCUpdateResult &UR) {
+PreservedAnalyses CoroAnnotationElidePass::run(Function &F,
+ FunctionAnalysisManager &FAM) {
bool Changed = false;
- CallGraphUpdater CGUpdater;
- CGUpdater.initialize(CG, C, AM, UR);
-
- auto &FAM =
- AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
-
- for (LazyCallGraph::Node &N : C) {
- Function *Callee = &N.getFunction();
- Function *NewCallee = Callee->getParent()->getFunction(
- (Callee->getName() + ".noalloc").str());
- if (!NewCallee) {
- continue;
- }
+
+ Function *NewCallee =
+ F.getParent()->getFunction((F.getName() + ".noalloc").str());
+ if (!NewCallee) {
+ return PreservedAnalyses::all();
+ }
auto FramePtrArgPosition = NewCallee->arg_size() - 1;
auto FrameSize =
@@ -122,34 +112,31 @@ PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
NewCallee->getParamAlign(FramePtrArgPosition).valueOrOne();
SmallVector<CallBase *, 4> Users;
- for (auto *U : Callee->users()) {
+ for (auto *U : F.users()) {
if (auto *CB = dyn_cast<CallBase>(U)) {
- if (CB->getCalledFunction() == Callee)
+ if (CB->getCalledFunction() == &F)
Users.push_back(CB);
}
}
- auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(*Callee);
+ auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
for (auto *CB : Users) {
auto *Caller = CB->getFunction();
if (Caller && Caller->isPresplitCoroutine() &&
CB->hasFnAttr(llvm::Attribute::CoroElideSafe)) {
- auto *CallerN = CG.lookup(*Caller);
- auto *CallerC = CG.lookupSCC(*CallerN);
processCall(CB, Caller, NewCallee, FrameSize, FrameAlign);
ORE.emit([&]() {
return OptimizationRemark(DEBUG_TYPE, "CoroAnnotationElide", Caller)
- << "'" << ore::NV("callee", Callee->getName())
- << "' elided in '" << ore::NV("caller", Caller->getName());
+ << "'" << ore::NV("callee", F.getName()) << "' elided in '"
+ << ore::NV("caller", Caller->getName());
});
+ FAM.invalidate(*Caller, PreservedAnalyses::none());
Changed = true;
- updateCGAndAnalysisManagerForCGSCCPass(CG, *CallerC, *CallerN, AM, UR,
- FAM);
}
}
- }
+
return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
}
More information about the llvm-commits
mailing list