Skip to content

Commit ce01a62

Browse files
aslasl
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 aae8931 commit ce01a62

8 files changed

+184
-71
lines changed

‎lib/IRGen/GenCall.cpp

+85-21
Original file line numberDiff line numberDiff line change
@@ -5718,25 +5718,15 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
57185718
auto &Builder = IGF.Builder;
57195719
auto &IGM = IGF.IGM;
57205720

5721-
// Create coroutine exit block and branch to it.
5722-
auto coroEndBB = IGF.createBasicBlock("coro.end.normal");
5723-
IGF.setCoroutineExitBlock(coroEndBB);
5724-
Builder.CreateBr(coroEndBB);
5725-
5726-
// Emit the block.
5727-
Builder.emitBlock(coroEndBB);
5728-
auto handle = IGF.getCoroutineHandle();
5729-
5730-
llvm::Value *resultToken = nullptr;
5721+
// Prepare coroutine result values
5722+
auto &coroResults = IGF.coroutineResults;
5723+
assert(coroResults.empty() && "must only be single return");
57315724
if (result.empty()) {
57325725
assert(IGM.getTypeInfo(returnResultType)
5733-
.nativeReturnValueSchema(IGM)
5734-
.empty() &&
5726+
.nativeReturnValueSchema(IGM)
5727+
.empty() &&
57355728
"Empty explosion must match the native calling convention");
5736-
// No results: just use none token
5737-
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
57385729
} else {
5739-
// Capture results via `coro_end_results` intrinsic
57405730
result = IGF.coerceValueTo(returnResultType, result, funcResultType);
57415731
auto &nativeSchema =
57425732
IGM.getTypeInfo(funcResultType).nativeReturnValueSchema(IGM);
@@ -5745,19 +5735,93 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
57455735
Explosion native = nativeSchema.mapIntoNative(IGM, IGF, result,
57465736
funcResultType,
57475737
false /* isOutlined */);
5748-
SmallVector<llvm::Value *, 1> args;
57495738
for (unsigned i = 0, e = native.size(); i != e; ++i)
5750-
args.push_back(native.claimNext());
5739+
coroResults.push_back(native.claimNext());
5740+
}
57515741

5752-
resultToken =
5753-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, args);
5742+
auto coroEndBB = IGF.getCoroutineExitBlock();
5743+
auto handle = IGF.getCoroutineHandle();
5744+
bool newEndBlock = false;
5745+
if (!coroEndBB) {
5746+
coroEndBB = IGF.createBasicBlock("coro.end");
5747+
IGF.setCoroutineExitBlock(coroEndBB);
5748+
newEndBlock = true;
57545749
}
57555750

5756-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
5751+
// If there are coroutine results, then we need to capture them via
5752+
// @llvm.coro_end_results intrinsics. However, since unwind blocks would
5753+
// jump to the same block, we wrap values into phi nodes.
5754+
Builder.CreateBr(coroEndBB);
5755+
5756+
// Emit the end block.
5757+
llvm::BasicBlock *returnBB = Builder.GetInsertBlock();
5758+
5759+
if (newEndBlock) {
5760+
Builder.emitBlock(coroEndBB);
5761+
5762+
llvm::Value *resultToken = nullptr;
5763+
if (coroResults.empty()) {
5764+
// No results: just use none token
5765+
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
5766+
} else {
5767+
// Otherwise, wrap result values into singleton phi nodes
5768+
for (auto &val : coroResults) {
5769+
auto *phi = Builder.CreatePHI(val->getType(), 0);
5770+
phi->addIncoming(val, returnBB);
5771+
val = phi;
5772+
}
5773+
5774+
// Capture results via result token
5775+
resultToken =
5776+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5777+
5778+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
57575779
{handle,
57585780
/*is unwind*/ Builder.getFalse(),
57595781
resultToken});
5760-
Builder.CreateUnreachable();
5782+
Builder.CreateUnreachable();
5783+
}
5784+
} else {
5785+
if (coroResults.empty()) {
5786+
// No results, we do not need to change anything around existing coro.end
5787+
return;
5788+
}
5789+
5790+
// Otherwise, we'd need to insert new coro.end.results intrinsics capturing
5791+
// result values. However, we'd need to wrap results into phi nodes adding
5792+
// undef for all values coming from incoming unwind blocks.
5793+
5794+
// Find coro.end intrinsic
5795+
llvm::CallInst *coroEndCall = nullptr;
5796+
for (llvm::Instruction &inst : coroEndBB->instructionsWithoutDebug()) {
5797+
if (auto *CI = dyn_cast<llvm::CallInst>(&inst)) {
5798+
if (CI->getIntrinsicID() == llvm::Intrinsic::coro_end) {
5799+
coroEndCall = CI;
5800+
break;
5801+
}
5802+
}
5803+
}
5804+
5805+
assert(coroEndCall && isa<llvm::ConstantTokenNone>(coroEndCall->getArgOperand(2)) &&
5806+
"invalid unwind coro.end call");
5807+
5808+
Builder.SetInsertPoint(&*coroEndBB->getFirstInsertionPt());
5809+
5810+
for (auto &val : coroResults) {
5811+
auto *phi = Builder.CreatePHI(val->getType(), llvm::pred_size(coroEndBB));
5812+
for (auto *predBB : llvm::predecessors(coroEndBB))
5813+
phi->addIncoming(predBB == returnBB ? val : llvm::UndefValue::get(val->getType()),
5814+
predBB);
5815+
5816+
val = phi;
5817+
}
5818+
5819+
// Capture results via result token and replace coro.end token operand
5820+
auto *resultToken =
5821+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5822+
coroEndCall->setArgOperand(2, resultToken);
5823+
Builder.SetInsertPoint(returnBB);
5824+
}
57615825
}
57625826

57635827
FunctionPointer

‎lib/IRGen/IRGenFunction.h

+2
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

+35-9
Original file line numberDiff line numberDiff line change
@@ -4141,20 +4141,46 @@ void IRGenSILFunction::visitUnreachableInst(swift::UnreachableInst *i) {
41414141
}
41424142

41434143
void IRGenFunction::emitCoroutineOrAsyncExit(bool isUnwind) {
4144-
// Create end block and branch to it.
4145-
auto coroEndBB = createBasicBlock(isUnwind ? "coro.end.unwind" : "coro.end");
4146-
Builder.CreateBr(coroEndBB);
4144+
// LLVM's retcon lowering is a bit imcompatible with Swift
4145+
// model. Essentially it assumes that unwind destination is kind of terminal -
4146+
// it cannot return back to caller and must somehow terminate the process /
4147+
// thread. Therefore we are always use normal LLVM coroutine termination.
4148+
// However, for yield_once coroutines we need also specify undef results on
4149+
// unwind path. Eventually, we'd get rid of these crazy phis...
4150+
4151+
// If the coroutine exit block already exists, just branch to it.
4152+
auto *coroEndBB = getCoroutineExitBlock();
4153+
auto *unwindBB = Builder.GetInsertBlock();
4154+
4155+
// If the coroutine exit block already exists, just branch to it.
4156+
if (coroEndBB) {
4157+
Builder.CreateBr(coroEndBB);
4158+
4159+
if (!isAsync()) {
4160+
// If there are any result values we need to add undefs for all values
4161+
// coming from unwind block
4162+
for (auto &phi : coroutineResults)
4163+
cast<llvm::PHINode>(phi)->addIncoming(llvm::UndefValue::get(phi->getType()),
4164+
unwindBB);
4165+
}
41474166

4148-
// Emit the block.
4167+
return;
4168+
}
4169+
4170+
// Otherwise, create it and branch to it.
4171+
coroEndBB = createBasicBlock("coro.end");
4172+
setCoroutineExitBlock(coroEndBB);
4173+
Builder.CreateBr(coroEndBB);
41494174
Builder.emitBlock(coroEndBB);
4175+
41504176
if (isAsync())
41514177
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_async,
4152-
{ getCoroutineHandle(),
4153-
isUnwind ? Builder.getTrue() : Builder.getFalse()});
4178+
{ getCoroutineHandle(), Builder.getFalse()});
41544179
else
4180+
// Do not bother about results here, normal result emission code would
4181+
// update token value.
41554182
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
4156-
{ getCoroutineHandle(),
4157-
isUnwind ? Builder.getTrue() : Builder.getFalse(),
4183+
{ getCoroutineHandle(), Builder.getFalse(),
41584184
llvm::ConstantTokenNone::get(Builder.getContext())});
41594185

41604186
Builder.CreateUnreachable();
@@ -4354,7 +4380,7 @@ void IRGenSILFunction::visitThrowAddrInst(swift::ThrowAddrInst *i) {
43544380
}
43554381

43564382
void IRGenSILFunction::visitUnwindInst(swift::UnwindInst *i) {
4357-
// Just call coro.end marking unwind return
4383+
// Call coro.end marking unwind return
43584384
emitCoroutineOrAsyncExit(true);
43594385
}
43604386

‎test/IRGen/yield_once.sil

+4-8
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

+8-16
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

+4-8
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

+4-8
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)