Skip to content

Commit 761bf33

Browse files
[LLVM][Coroutines] Switch CoroAnnotationElidePass to a FunctionPass (#107897)
After landing #99285 we found that the call graph update was causing the following crash when expensive checks are turned on ``` 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. ``` I have to admit I believe that the call graph update process I did for that patch could be wrong. 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.
1 parent 7a8e9df commit 761bf33

File tree

4 files changed

+22
-39
lines changed

4 files changed

+22
-39
lines changed

llvm/include/llvm/Transforms/Coroutines/CoroAnnotationElide.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,18 +17,14 @@
1717
#ifndef LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
1818
#define LLVM_TRANSFORMS_COROUTINES_COROANNOTATIONELIDE_H
1919

20-
#include "llvm/Analysis/CGSCCPassManager.h"
21-
#include "llvm/Analysis/LazyCallGraph.h"
2220
#include "llvm/IR/PassManager.h"
2321

2422
namespace llvm {
2523

26-
struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
27-
CoroAnnotationElidePass() {}
28-
29-
PreservedAnalyses run(LazyCallGraph::SCC &C, CGSCCAnalysisManager &AM,
30-
LazyCallGraph &CG, CGSCCUpdateResult &UR);
24+
class Function;
3125

26+
struct CoroAnnotationElidePass : PassInfoMixin<CoroAnnotationElidePass> {
27+
PreservedAnalyses run(Function &F, FunctionAnalysisManager &FAM);
3228
static bool isRequired() { return false; }
3329
};
3430
} // end namespace llvm

llvm/lib/Passes/PassBuilderPipelines.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,8 @@ PassBuilder::buildInlinerPipeline(OptimizationLevel Level,
977977

978978
if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
979979
MainCGPipeline.addPass(CoroSplitPass(Level != OptimizationLevel::O0));
980-
MainCGPipeline.addPass(CoroAnnotationElidePass());
980+
MainCGPipeline.addPass(
981+
createCGSCCToFunctionPassAdaptor(CoroAnnotationElidePass()));
981982
}
982983

983984
// Make sure we don't affect potential future NoRerun CGSCC adaptors.
@@ -1028,8 +1029,7 @@ PassBuilder::buildModuleInlinerPipeline(OptimizationLevel Level,
10281029
if (Phase != ThinOrFullLTOPhase::ThinLTOPreLink) {
10291030
MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(
10301031
CoroSplitPass(Level != OptimizationLevel::O0)));
1031-
MPM.addPass(
1032-
createModuleToPostOrderCGSCCPassAdaptor(CoroAnnotationElidePass()));
1032+
MPM.addPass(createModuleToFunctionPassAdaptor(CoroAnnotationElidePass()));
10331033
}
10341034

10351035
return MPM;

llvm/lib/Passes/PassRegistry.def

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,6 @@ CGSCC_PASS("attributor-light-cgscc", AttributorLightCGSCCPass())
243243
CGSCC_PASS("invalidate<all>", InvalidateAllAnalysesPass())
244244
CGSCC_PASS("no-op-cgscc", NoOpCGSCCPass())
245245
CGSCC_PASS("openmp-opt-cgscc", OpenMPOptCGSCCPass())
246-
CGSCC_PASS("coro-annotation-elide", CoroAnnotationElidePass())
247246
#undef CGSCC_PASS
248247

249248
#ifndef CGSCC_PASS_WITH_PARAMS
@@ -343,6 +342,7 @@ FUNCTION_PASS("codegenprepare", CodeGenPreparePass(TM))
343342
FUNCTION_PASS("consthoist", ConstantHoistingPass())
344343
FUNCTION_PASS("constraint-elimination", ConstraintEliminationPass())
345344
FUNCTION_PASS("coro-elide", CoroElidePass())
345+
FUNCTION_PASS("coro-annotation-elide", CoroAnnotationElidePass())
346346
FUNCTION_PASS("correlated-propagation", CorrelatedValuePropagationPass())
347347
FUNCTION_PASS("count-visits", CountVisitsPass())
348348
FUNCTION_PASS("dce", DCEPass())

llvm/lib/Transforms/Coroutines/CoroAnnotationElide.cpp

Lines changed: 15 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
#include "llvm/Transforms/Coroutines/CoroAnnotationElide.h"
1818

19-
#include "llvm/Analysis/CGSCCPassManager.h"
2019
#include "llvm/Analysis/LazyCallGraph.h"
2120
#include "llvm/Analysis/OptimizationRemarkEmitter.h"
2221
#include "llvm/IR/Analysis.h"
@@ -96,24 +95,15 @@ static void processCall(CallBase *CB, Function *Caller, Function *NewCallee,
9695
CB->eraseFromParent();
9796
}
9897

99-
PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
100-
CGSCCAnalysisManager &AM,
101-
LazyCallGraph &CG,
102-
CGSCCUpdateResult &UR) {
98+
PreservedAnalyses CoroAnnotationElidePass::run(Function &F,
99+
FunctionAnalysisManager &FAM) {
103100
bool Changed = false;
104-
CallGraphUpdater CGUpdater;
105-
CGUpdater.initialize(CG, C, AM, UR);
106-
107-
auto &FAM =
108-
AM.getResult<FunctionAnalysisManagerCGSCCProxy>(C, CG).getManager();
109-
110-
for (LazyCallGraph::Node &N : C) {
111-
Function *Callee = &N.getFunction();
112-
Function *NewCallee = Callee->getParent()->getFunction(
113-
(Callee->getName() + ".noalloc").str());
114-
if (!NewCallee) {
115-
continue;
116-
}
101+
102+
Function *NewCallee =
103+
F.getParent()->getFunction((F.getName() + ".noalloc").str());
104+
if (!NewCallee) {
105+
return PreservedAnalyses::all();
106+
}
117107

118108
auto FramePtrArgPosition = NewCallee->arg_size() - 1;
119109
auto FrameSize =
@@ -122,34 +112,31 @@ PreservedAnalyses CoroAnnotationElidePass::run(LazyCallGraph::SCC &C,
122112
NewCallee->getParamAlign(FramePtrArgPosition).valueOrOne();
123113

124114
SmallVector<CallBase *, 4> Users;
125-
for (auto *U : Callee->users()) {
115+
for (auto *U : F.users()) {
126116
if (auto *CB = dyn_cast<CallBase>(U)) {
127-
if (CB->getCalledFunction() == Callee)
117+
if (CB->getCalledFunction() == &F)
128118
Users.push_back(CB);
129119
}
130120
}
131121

132-
auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(*Callee);
122+
auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
133123

134124
for (auto *CB : Users) {
135125
auto *Caller = CB->getFunction();
136126
if (Caller && Caller->isPresplitCoroutine() &&
137127
CB->hasFnAttr(llvm::Attribute::CoroElideSafe)) {
138128

139-
auto *CallerN = CG.lookup(*Caller);
140-
auto *CallerC = CG.lookupSCC(*CallerN);
141129
processCall(CB, Caller, NewCallee, FrameSize, FrameAlign);
142130

143131
ORE.emit([&]() {
144132
return OptimizationRemark(DEBUG_TYPE, "CoroAnnotationElide", Caller)
145-
<< "'" << ore::NV("callee", Callee->getName())
146-
<< "' elided in '" << ore::NV("caller", Caller->getName());
133+
<< "'" << ore::NV("callee", F.getName()) << "' elided in '"
134+
<< ore::NV("caller", Caller->getName());
147135
});
136+
FAM.invalidate(*Caller, PreservedAnalyses::none());
148137
Changed = true;
149-
updateCGAndAnalysisManagerForCGSCCPass(CG, *CallerC, *CallerN, AM, UR,
150-
FAM);
151138
}
152139
}
153-
}
140+
154141
return Changed ? PreservedAnalyses::none() : PreservedAnalyses::all();
155142
}

0 commit comments

Comments
 (0)