Skip to content

Commit f18d679

Browse files
aslasavonic
asl
authored and
asavonic
committed
Workaround LLVM coroutine codegen problem: it assumes that unwind path never returns.
This is not true to Swift coroutines as unwind path should end with error result.
1 parent b789147 commit f18d679

File tree

8 files changed

+184
-71
lines changed

8 files changed

+184
-71
lines changed

‎lib/IRGen/GenCall.cpp

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5829,25 +5829,15 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
58295829
auto &Builder = IGF.Builder;
58305830
auto &IGM = IGF.IGM;
58315831

5832-
// Create coroutine exit block and branch to it.
5833-
auto coroEndBB = IGF.createBasicBlock("coro.end.normal");
5834-
IGF.setCoroutineExitBlock(coroEndBB);
5835-
Builder.CreateBr(coroEndBB);
5836-
5837-
// Emit the block.
5838-
Builder.emitBlock(coroEndBB);
5839-
auto handle = IGF.getCoroutineHandle();
5840-
5841-
llvm::Value *resultToken = nullptr;
5832+
// Prepare coroutine result values
5833+
auto &coroResults = IGF.coroutineResults;
5834+
assert(coroResults.empty() && "must only be single return");
58425835
if (result.empty()) {
58435836
assert(IGM.getTypeInfo(returnResultType)
5844-
.nativeReturnValueSchema(IGM)
5845-
.empty() &&
5837+
.nativeReturnValueSchema(IGM)
5838+
.empty() &&
58465839
"Empty explosion must match the native calling convention");
5847-
// No results: just use none token
5848-
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
58495840
} else {
5850-
// Capture results via `coro_end_results` intrinsic
58515841
result = IGF.coerceValueTo(returnResultType, result, funcResultType);
58525842
auto &nativeSchema =
58535843
IGM.getTypeInfo(funcResultType).nativeReturnValueSchema(IGM);
@@ -5856,19 +5846,93 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
58565846
Explosion native = nativeSchema.mapIntoNative(IGM, IGF, result,
58575847
funcResultType,
58585848
false /* isOutlined */);
5859-
SmallVector<llvm::Value *, 1> args;
58605849
for (unsigned i = 0, e = native.size(); i != e; ++i)
5861-
args.push_back(native.claimNext());
5850+
coroResults.push_back(native.claimNext());
5851+
}
58625852

5863-
resultToken =
5864-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, args);
5853+
auto coroEndBB = IGF.getCoroutineExitBlock();
5854+
auto handle = IGF.getCoroutineHandle();
5855+
bool newEndBlock = false;
5856+
if (!coroEndBB) {
5857+
coroEndBB = IGF.createBasicBlock("coro.end");
5858+
IGF.setCoroutineExitBlock(coroEndBB);
5859+
newEndBlock = true;
58655860
}
58665861

5867-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
5862+
// If there are coroutine results, then we need to capture them via
5863+
// @llvm.coro_end_results intrinsics. However, since unwind blocks would
5864+
// jump to the same block, we wrap values into phi nodes.
5865+
Builder.CreateBr(coroEndBB);
5866+
5867+
// Emit the end block.
5868+
llvm::BasicBlock *returnBB = Builder.GetInsertBlock();
5869+
5870+
if (newEndBlock) {
5871+
Builder.emitBlock(coroEndBB);
5872+
5873+
llvm::Value *resultToken = nullptr;
5874+
if (coroResults.empty()) {
5875+
// No results: just use none token
5876+
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
5877+
} else {
5878+
// Otherwise, wrap result values into singleton phi nodes
5879+
for (auto &val : coroResults) {
5880+
auto *phi = Builder.CreatePHI(val->getType(), 0);
5881+
phi->addIncoming(val, returnBB);
5882+
val = phi;
5883+
}
5884+
5885+
// Capture results via result token
5886+
resultToken =
5887+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5888+
5889+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
58685890
{handle,
58695891
/*is unwind*/ Builder.getFalse(),
58705892
resultToken});
5871-
Builder.CreateUnreachable();
5893+
Builder.CreateUnreachable();
5894+
}
5895+
} else {
5896+
if (coroResults.empty()) {
5897+
// No results, we do not need to change anything around existing coro.end
5898+
return;
5899+
}
5900+
5901+
// Otherwise, we'd need to insert new coro.end.results intrinsics capturing
5902+
// result values. However, we'd need to wrap results into phi nodes adding
5903+
// undef for all values coming from incoming unwind blocks.
5904+
5905+
// Find coro.end intrinsic
5906+
llvm::CallInst *coroEndCall = nullptr;
5907+
for (llvm::Instruction &inst : coroEndBB->instructionsWithoutDebug()) {
5908+
if (auto *CI = dyn_cast<llvm::CallInst>(&inst)) {
5909+
if (CI->getIntrinsicID() == llvm::Intrinsic::coro_end) {
5910+
coroEndCall = CI;
5911+
break;
5912+
}
5913+
}
5914+
}
5915+
5916+
assert(coroEndCall && isa<llvm::ConstantTokenNone>(coroEndCall->getArgOperand(2)) &&
5917+
"invalid unwind coro.end call");
5918+
5919+
Builder.SetInsertPoint(&*coroEndBB->getFirstInsertionPt());
5920+
5921+
for (auto &val : coroResults) {
5922+
auto *phi = Builder.CreatePHI(val->getType(), llvm::pred_size(coroEndBB));
5923+
for (auto *predBB : llvm::predecessors(coroEndBB))
5924+
phi->addIncoming(predBB == returnBB ? val : llvm::UndefValue::get(val->getType()),
5925+
predBB);
5926+
5927+
val = phi;
5928+
}
5929+
5930+
// Capture results via result token and replace coro.end token operand
5931+
auto *resultToken =
5932+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5933+
coroEndCall->setArgOperand(2, resultToken);
5934+
Builder.SetInsertPoint(returnBB);
5935+
}
58725936
}
58735937

58745938
FunctionPointer

‎lib/IRGen/IRGenFunction.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ class IRGenFunction {
159159
return CoroutineExitBlock;
160160
}
161161

162+
SmallVector<llvm::Value *, 1> coroutineResults;
163+
162164
void setCoroutineExitBlock(llvm::BasicBlock *block) {
163165
assert(CoroutineExitBlock == nullptr && "already set exit BB");
164166
assert(block != nullptr && "setting a null exit BB");

‎lib/IRGen/IRGenSIL.cpp

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4122,20 +4122,46 @@ void IRGenSILFunction::visitUnreachableInst(swift::UnreachableInst *i) {
41224122
}
41234123

41244124
void IRGenFunction::emitCoroutineOrAsyncExit(bool isUnwind) {
4125-
// Create end block and branch to it.
4126-
auto coroEndBB = createBasicBlock(isUnwind ? "coro.end.unwind" : "coro.end");
4127-
Builder.CreateBr(coroEndBB);
4125+
// LLVM's retcon lowering is a bit imcompatible with Swift
4126+
// model. Essentially it assumes that unwind destination is kind of terminal -
4127+
// it cannot return back to caller and must somehow terminate the process /
4128+
// thread. Therefore we are always use normal LLVM coroutine termination.
4129+
// However, for yield_once coroutines we need also specify undef results on
4130+
// unwind path. Eventually, we'd get rid of these crazy phis...
4131+
4132+
// If the coroutine exit block already exists, just branch to it.
4133+
auto *coroEndBB = getCoroutineExitBlock();
4134+
auto *unwindBB = Builder.GetInsertBlock();
4135+
4136+
// If the coroutine exit block already exists, just branch to it.
4137+
if (coroEndBB) {
4138+
Builder.CreateBr(coroEndBB);
4139+
4140+
if (!isAsync()) {
4141+
// If there are any result values we need to add undefs for all values
4142+
// coming from unwind block
4143+
for (auto &phi : coroutineResults)
4144+
cast<llvm::PHINode>(phi)->addIncoming(llvm::UndefValue::get(phi->getType()),
4145+
unwindBB);
4146+
}
41284147

4129-
// Emit the block.
4148+
return;
4149+
}
4150+
4151+
// Otherwise, create it and branch to it.
4152+
coroEndBB = createBasicBlock("coro.end");
4153+
setCoroutineExitBlock(coroEndBB);
4154+
Builder.CreateBr(coroEndBB);
41304155
Builder.emitBlock(coroEndBB);
4156+
41314157
if (isAsync())
41324158
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_async,
4133-
{ getCoroutineHandle(),
4134-
isUnwind ? Builder.getTrue() : Builder.getFalse()});
4159+
{ getCoroutineHandle(), Builder.getFalse()});
41354160
else
4161+
// Do not bother about results here, normal result emission code would
4162+
// update token value.
41364163
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
4137-
{ getCoroutineHandle(),
4138-
isUnwind ? Builder.getTrue() : Builder.getFalse(),
4164+
{ getCoroutineHandle(), Builder.getFalse(),
41394165
llvm::ConstantTokenNone::get(Builder.getContext())});
41404166

41414167
Builder.CreateUnreachable();
@@ -4335,7 +4361,7 @@ void IRGenSILFunction::visitThrowAddrInst(swift::ThrowAddrInst *i) {
43354361
}
43364362

43374363
void IRGenSILFunction::visitUnwindInst(swift::UnwindInst *i) {
4338-
// Just call coro.end marking unwind return
4364+
// Call coro.end marking unwind return
43394365
emitCoroutineOrAsyncExit(true);
43404366
}
43414367

‎test/IRGen/yield_once.sil

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,23 +27,19 @@ resume:
2727
// CHECK: call swiftcc void @marker(i32 2000)
2828
%2000 = integer_literal $Builtin.Int32, 2000
2929
apply %marker(%2000) : $@convention(thin) (Builtin.Int32) -> ()
30-
// CHECK: br label %coro.end.normal
30+
// CHECK: br label %coro.end
3131
%ret = tuple ()
3232
return %ret : $()
3333

34-
// CHECK: coro.end.normal:
35-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
36-
// CHECK-NEXT: unreachable
37-
3834
unwind:
3935
// CHECK: call swiftcc void @marker(i32 3000)
4036
%3000 = integer_literal $Builtin.Int32, 3000
4137
apply %marker(%3000) : $@convention(thin) (Builtin.Int32) -> ()
42-
// CHECK: br label %coro.end.unwind
38+
// CHECK: br label %coro.end
4339
unwind
4440

45-
// CHECK: coro.end.unwind:
46-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 true, token none)
41+
// CHECK: coro.end:
42+
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
4743
// CHECK-NEXT: unreachable
4844
}
4945

‎test/IRGen/yield_once_big.sil

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -72,24 +72,20 @@ resume:
7272
%2000 = integer_literal $Builtin.Int32, 2000
7373
apply %marker(%2000) : $@convention(thin) (Builtin.Int32) -> ()
7474
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
75-
// CHECK-NEXT: br label %coro.end.normal
75+
// CHECK-NEXT: br label %coro.end
7676
%ret = tuple ()
7777
return %ret : $()
7878

79-
// CHECK: coro.end.normal:
80-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
81-
// CHECK-NEXT: unreachable
82-
8379
unwind:
8480
// CHECK: call swiftcc void @marker(i32 3000)
8581
%3000 = integer_literal $Builtin.Int32, 3000
8682
apply %marker(%3000) : $@convention(thin) (Builtin.Int32) -> ()
8783
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
88-
// CHECK-NEXT: br label %coro.end.unwind
84+
// CHECK-NEXT: br label %coro.end
8985
unwind
9086

91-
// CHECK: coro.end.unwind:
92-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 true, token none)
87+
// CHECK: coro.end:
88+
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
9389
// CHECK-NEXT: unreachable
9490
}
9591

@@ -223,24 +219,20 @@ resume:
223219
%2000 = integer_literal $Builtin.Int32, 2000
224220
apply %marker(%2000) : $@convention(thin) (Builtin.Int32) -> ()
225221
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
226-
// CHECK-NEXT: br label %coro.end.normal
222+
// CHECK-NEXT: br label %coro.end
227223
%ret = tuple ()
228224
return %ret : $()
229225

230-
// CHECK: coro.end.normal:
231-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
232-
// CHECK-NEXT: unreachable
233-
234226
unwind:
235227
end_borrow %value : $BigWrapper<C>
236228
// CHECK: call swiftcc void @marker(i32 3000)
237229
%3000 = integer_literal $Builtin.Int32, 3000
238230
apply %marker(%3000) : $@convention(thin) (Builtin.Int32) -> ()
239231
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
240-
// CHECK-NEXT: br label %coro.end.unwind
232+
// CHECK-NEXT: br label %coro.end
241233
unwind
242234

243-
// CHECK: coro.end.unwind:
244-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 true, token none)
235+
// CHECK: coro.end:
236+
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
245237
// CHECK-NEXT: unreachable
246238
}

‎test/IRGen/yield_once_biggish.sil

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -77,23 +77,19 @@ resume:
7777
// CHECK: call swiftcc void @marker(i32 2000)
7878
%2000 = integer_literal $Builtin.Int32, 2000
7979
apply %marker(%2000) : $@convention(thin) (Builtin.Int32) -> ()
80-
// CHECK: br label %coro.end.normal
80+
// CHECK: br label %coro.end
8181
%ret = tuple ()
8282
return %ret : $()
8383

84-
// CHECK: coro.end.normal:
85-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
86-
// CHECK-NEXT: unreachable
87-
8884
unwind:
8985
// CHECK: call swiftcc void @marker(i32 3000)
9086
%3000 = integer_literal $Builtin.Int32, 3000
9187
apply %marker(%3000) : $@convention(thin) (Builtin.Int32) -> ()
92-
// CHECK: br label %coro.end.unwind
88+
// CHECK: br label %coro.end
9389
unwind
9490

95-
// CHECK: coro.end.unwind:
96-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 true, token none)
91+
// CHECK: coro.end:
92+
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
9793
// CHECK-NEXT: unreachable
9894
}
9995

‎test/IRGen/yield_once_indirect.sil

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,10 @@ resume:
6060
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
6161
dealloc_stack %temp : $*Indirect<C>
6262

63-
// CHECK-NEXT: br label %coro.end.normal
63+
// CHECK-NEXT: br label %coro.end
6464
%ret = tuple ()
6565
return %ret : $()
6666

67-
// CHECK: coro.end.normal:
68-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
69-
// CHECK-NEXT: unreachable
70-
7167
unwind:
7268
// CHECK: call swiftcc void @marker(i32 3000)
7369
%3000 = integer_literal $Builtin.Int32, 3000
@@ -76,11 +72,11 @@ unwind:
7672
// CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 {{.*}}, ptr [[TEMP]])
7773
dealloc_stack %temp : $*Indirect<C>
7874

79-
// CHECK-NEXT: br label %coro.end.unwind
75+
// CHECK-NEXT: br label %coro.end
8076
unwind
8177

82-
// CHECK: coro.end.unwind:
83-
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 true, token none)
78+
// CHECK: coro.end:
79+
// CHECK: call i1 @llvm.coro.end(ptr [[BEGIN]], i1 false, token none)
8480
// CHECK-NEXT: unreachable
8581
}
8682

0 commit comments

Comments
 (0)