Skip to content

Commit 853bff2

Browse files
[Coroutines] properly update CallGraph in CoroSplit (#107935)
Fixes #107139. We weren't updating the call graph properly in CoroSplit. This crash is due to the await_suspend() function calling the coroutine, forming a multi-node SCC. The issue bisected to #79712 but I think this is red herring. We haven't been properly updating the call graph. Added an example of such code as a test case.
1 parent 5d17293 commit 853bff2

File tree

2 files changed

+50
-6
lines changed

2 files changed

+50
-6
lines changed

llvm/lib/Transforms/Coroutines/CoroSplit.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,12 +2080,13 @@ splitCoroutine(Function &F, SmallVectorImpl<Function *> &Clones,
20802080
return Shape;
20812081
}
20822082

2083-
static void updateCallGraphAfterCoroutineSplit(
2083+
static LazyCallGraph::SCC &updateCallGraphAfterCoroutineSplit(
20842084
LazyCallGraph::Node &N, const coro::Shape &Shape,
20852085
const SmallVectorImpl<Function *> &Clones, LazyCallGraph::SCC &C,
20862086
LazyCallGraph &CG, CGSCCAnalysisManager &AM, CGSCCUpdateResult &UR,
20872087
FunctionAnalysisManager &FAM) {
20882088

2089+
auto *CurrentSCC = &C;
20892090
if (!Clones.empty()) {
20902091
switch (Shape.ABI) {
20912092
case coro::ABI::Switch:
@@ -2105,13 +2106,16 @@ static void updateCallGraphAfterCoroutineSplit(
21052106
}
21062107

21072108
// Let the CGSCC infra handle the changes to the original function.
2108-
updateCGAndAnalysisManagerForCGSCCPass(CG, C, N, AM, UR, FAM);
2109+
CurrentSCC = &updateCGAndAnalysisManagerForCGSCCPass(CG, *CurrentSCC, N, AM,
2110+
UR, FAM);
21092111
}
21102112

21112113
// Do some cleanup and let the CGSCC infra see if we've cleaned up any edges
21122114
// to the split functions.
21132115
postSplitCleanup(N.getFunction());
2114-
updateCGAndAnalysisManagerForFunctionPass(CG, C, N, AM, UR, FAM);
2116+
CurrentSCC = &updateCGAndAnalysisManagerForFunctionPass(CG, *CurrentSCC, N,
2117+
AM, UR, FAM);
2118+
return *CurrentSCC;
21152119
}
21162120

21172121
/// Replace a call to llvm.coro.prepare.retcon.
@@ -2200,6 +2204,7 @@ PreservedAnalyses CoroSplitPass::run(LazyCallGraph::SCC &C,
22002204
if (Coroutines.empty() && PrepareFns.empty())
22012205
return PreservedAnalyses::all();
22022206

2207+
auto *CurrentSCC = &C;
22032208
// Split all the coroutines.
22042209
for (LazyCallGraph::Node *N : Coroutines) {
22052210
Function &F = N->getFunction();
@@ -2211,7 +2216,8 @@ PreservedAnalyses CoroSplitPass::run(LazyCallGraph::SCC &C,
22112216
coro::Shape Shape =
22122217
splitCoroutine(F, Clones, FAM.getResult<TargetIRAnalysis>(F),
22132218
OptimizeFrame, MaterializableCallback);
2214-
updateCallGraphAfterCoroutineSplit(*N, Shape, Clones, C, CG, AM, UR, FAM);
2219+
CurrentSCC = &updateCallGraphAfterCoroutineSplit(
2220+
*N, Shape, Clones, *CurrentSCC, CG, AM, UR, FAM);
22152221

22162222
auto &ORE = FAM.getResult<OptimizationRemarkEmitterAnalysis>(F);
22172223
ORE.emit([&]() {
@@ -2223,14 +2229,14 @@ PreservedAnalyses CoroSplitPass::run(LazyCallGraph::SCC &C,
22232229

22242230
if (!Shape.CoroSuspends.empty()) {
22252231
// Run the CGSCC pipeline on the original and newly split functions.
2226-
UR.CWorklist.insert(&C);
2232+
UR.CWorklist.insert(CurrentSCC);
22272233
for (Function *Clone : Clones)
22282234
UR.CWorklist.insert(CG.lookupSCC(CG.get(*Clone)));
22292235
}
22302236
}
22312237

22322238
for (auto *PrepareFn : PrepareFns) {
2233-
replaceAllPrepares(PrepareFn, CG, C);
2239+
replaceAllPrepares(PrepareFn, CG, *CurrentSCC);
22342240
}
22352241

22362242
return PreservedAnalyses::none();
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
; Verify that we don't crash on mutually recursive coroutines
2+
; RUN: opt < %s -passes='cgscc(coro-split)' -S | FileCheck %s
3+
4+
target triple = "x86_64-redhat-linux-gnu"
5+
6+
; CHECK-LABEL: define void @foo
7+
define void @foo() presplitcoroutine personality ptr null {
8+
entry:
9+
10+
%0 = call token @llvm.coro.id(i32 0, ptr null, ptr null, ptr null)
11+
%1 = call ptr @llvm.coro.begin(token %0, ptr null)
12+
%2 = call token @llvm.coro.save(ptr null)
13+
%3 = call i8 @llvm.coro.suspend(token none, i1 false)
14+
%4 = call token @llvm.coro.save(ptr null)
15+
; CHECK: call void @bar(ptr null, ptr null)
16+
call void @llvm.coro.await.suspend.void(ptr null, ptr null, ptr @bar)
17+
ret void
18+
}
19+
20+
; CHECK-LABEL: define void @bar({{.*}})
21+
define void @bar(ptr %0, ptr %1) {
22+
entry:
23+
; CHECK: call void @foo()
24+
call void @foo()
25+
ret void
26+
}
27+
28+
; CHECK-LABEL: @foo.resume({{.*}})
29+
; CHECK-LABEL: @foo.destroy({{.*}})
30+
; CHECK-LABEL: @foo.cleanup({{.*}})
31+
32+
declare token @llvm.coro.id(i32, ptr readnone, ptr nocapture readonly, ptr) #0
33+
declare ptr @llvm.coro.begin(token, ptr writeonly) nounwind
34+
declare token @llvm.coro.save(ptr) nomerge nounwind
35+
declare void @llvm.coro.await.suspend.void(ptr, ptr, ptr)
36+
declare i8 @llvm.coro.suspend(token, i1) nounwind
37+
38+
attributes #0 = { nocallback nofree nosync nounwind willreturn memory(argmem: read) }

0 commit comments

Comments
 (0)