Skip to content

Commit ad6751c

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 d4bf0c3 commit ad6751c

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
@@ -5865,25 +5865,15 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
58655865
auto &Builder = IGF.Builder;
58665866
auto &IGM = IGF.IGM;
58675867

5868-
// Create coroutine exit block and branch to it.
5869-
auto coroEndBB = IGF.createBasicBlock("coro.end.normal");
5870-
IGF.setCoroutineExitBlock(coroEndBB);
5871-
Builder.CreateBr(coroEndBB);
5872-
5873-
// Emit the block.
5874-
Builder.emitBlock(coroEndBB);
5875-
auto handle = IGF.getCoroutineHandle();
5876-
5877-
llvm::Value *resultToken = nullptr;
5868+
// Prepare coroutine result values
5869+
auto &coroResults = IGF.coroutineResults;
5870+
assert(coroResults.empty() && "must only be single return");
58785871
if (result.empty()) {
58795872
assert(IGM.getTypeInfo(returnResultType)
5880-
.nativeReturnValueSchema(IGM)
5881-
.empty() &&
5873+
.nativeReturnValueSchema(IGM)
5874+
.empty() &&
58825875
"Empty explosion must match the native calling convention");
5883-
// No results: just use none token
5884-
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
58855876
} else {
5886-
// Capture results via `coro_end_results` intrinsic
58875877
result = IGF.coerceValueTo(returnResultType, result, funcResultType);
58885878
auto &nativeSchema =
58895879
IGM.getTypeInfo(funcResultType).nativeReturnValueSchema(IGM);
@@ -5892,19 +5882,93 @@ void irgen::emitYieldOnceCoroutineResult(IRGenFunction &IGF, Explosion &result,
58925882
Explosion native = nativeSchema.mapIntoNative(IGM, IGF, result,
58935883
funcResultType,
58945884
false /* isOutlined */);
5895-
SmallVector<llvm::Value *, 1> args;
58965885
for (unsigned i = 0, e = native.size(); i != e; ++i)
5897-
args.push_back(native.claimNext());
5886+
coroResults.push_back(native.claimNext());
5887+
}
58985888

5899-
resultToken =
5900-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, args);
5889+
auto coroEndBB = IGF.getCoroutineExitBlock();
5890+
auto handle = IGF.getCoroutineHandle();
5891+
bool newEndBlock = false;
5892+
if (!coroEndBB) {
5893+
coroEndBB = IGF.createBasicBlock("coro.end");
5894+
IGF.setCoroutineExitBlock(coroEndBB);
5895+
newEndBlock = true;
59015896
}
59025897

5903-
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
5898+
// If there are coroutine results, then we need to capture them via
5899+
// @llvm.coro_end_results intrinsics. However, since unwind blocks would
5900+
// jump to the same block, we wrap values into phi nodes.
5901+
Builder.CreateBr(coroEndBB);
5902+
5903+
// Emit the end block.
5904+
llvm::BasicBlock *returnBB = Builder.GetInsertBlock();
5905+
5906+
if (newEndBlock) {
5907+
Builder.emitBlock(coroEndBB);
5908+
5909+
llvm::Value *resultToken = nullptr;
5910+
if (coroResults.empty()) {
5911+
// No results: just use none token
5912+
resultToken = llvm::ConstantTokenNone::get(Builder.getContext());
5913+
} else {
5914+
// Otherwise, wrap result values into singleton phi nodes
5915+
for (auto &val : coroResults) {
5916+
auto *phi = Builder.CreatePHI(val->getType(), 0);
5917+
phi->addIncoming(val, returnBB);
5918+
val = phi;
5919+
}
5920+
5921+
// Capture results via result token
5922+
resultToken =
5923+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5924+
5925+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
59045926
{handle,
59055927
/*is unwind*/ Builder.getFalse(),
59065928
resultToken});
5907-
Builder.CreateUnreachable();
5929+
Builder.CreateUnreachable();
5930+
}
5931+
} else {
5932+
if (coroResults.empty()) {
5933+
// No results, we do not need to change anything around existing coro.end
5934+
return;
5935+
}
5936+
5937+
// Otherwise, we'd need to insert new coro.end.results intrinsics capturing
5938+
// result values. However, we'd need to wrap results into phi nodes adding
5939+
// undef for all values coming from incoming unwind blocks.
5940+
5941+
// Find coro.end intrinsic
5942+
llvm::CallInst *coroEndCall = nullptr;
5943+
for (llvm::Instruction &inst : coroEndBB->instructionsWithoutDebug()) {
5944+
if (auto *CI = dyn_cast<llvm::CallInst>(&inst)) {
5945+
if (CI->getIntrinsicID() == llvm::Intrinsic::coro_end) {
5946+
coroEndCall = CI;
5947+
break;
5948+
}
5949+
}
5950+
}
5951+
5952+
assert(coroEndCall && isa<llvm::ConstantTokenNone>(coroEndCall->getArgOperand(2)) &&
5953+
"invalid unwind coro.end call");
5954+
5955+
Builder.SetInsertPoint(&*coroEndBB->getFirstInsertionPt());
5956+
5957+
for (auto &val : coroResults) {
5958+
auto *phi = Builder.CreatePHI(val->getType(), llvm::pred_size(coroEndBB));
5959+
for (auto *predBB : llvm::predecessors(coroEndBB))
5960+
phi->addIncoming(predBB == returnBB ? val : llvm::UndefValue::get(val->getType()),
5961+
predBB);
5962+
5963+
val = phi;
5964+
}
5965+
5966+
// Capture results via result token and replace coro.end token operand
5967+
auto *resultToken =
5968+
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_results, coroResults);
5969+
coroEndCall->setArgOperand(2, resultToken);
5970+
Builder.SetInsertPoint(returnBB);
5971+
}
59085972
}
59095973

59105974
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
@@ -4098,20 +4098,46 @@ void IRGenSILFunction::visitUnreachableInst(swift::UnreachableInst *i) {
40984098
}
40994099

41004100
void IRGenFunction::emitCoroutineOrAsyncExit(bool isUnwind) {
4101-
// Create end block and branch to it.
4102-
auto coroEndBB = createBasicBlock(isUnwind ? "coro.end.unwind" : "coro.end");
4103-
Builder.CreateBr(coroEndBB);
4101+
// LLVM's retcon lowering is a bit imcompatible with Swift
4102+
// model. Essentially it assumes that unwind destination is kind of terminal -
4103+
// it cannot return back to caller and must somehow terminate the process /
4104+
// thread. Therefore we are always use normal LLVM coroutine termination.
4105+
// However, for yield_once coroutines we need also specify undef results on
4106+
// unwind path. Eventually, we'd get rid of these crazy phis...
4107+
4108+
// If the coroutine exit block already exists, just branch to it.
4109+
auto *coroEndBB = getCoroutineExitBlock();
4110+
auto *unwindBB = Builder.GetInsertBlock();
4111+
4112+
// If the coroutine exit block already exists, just branch to it.
4113+
if (coroEndBB) {
4114+
Builder.CreateBr(coroEndBB);
4115+
4116+
if (!isAsync()) {
4117+
// If there are any result values we need to add undefs for all values
4118+
// coming from unwind block
4119+
for (auto &phi : coroutineResults)
4120+
cast<llvm::PHINode>(phi)->addIncoming(llvm::UndefValue::get(phi->getType()),
4121+
unwindBB);
4122+
}
41044123

4105-
// Emit the block.
4124+
return;
4125+
}
4126+
4127+
// Otherwise, create it and branch to it.
4128+
coroEndBB = createBasicBlock("coro.end");
4129+
setCoroutineExitBlock(coroEndBB);
4130+
Builder.CreateBr(coroEndBB);
41064131
Builder.emitBlock(coroEndBB);
4132+
41074133
if (isAsync())
41084134
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_async,
4109-
{ getCoroutineHandle(),
4110-
isUnwind ? Builder.getTrue() : Builder.getFalse()});
4135+
{ getCoroutineHandle(), Builder.getFalse()});
41114136
else
4137+
// Do not bother about results here, normal result emission code would
4138+
// update token value.
41124139
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
4113-
{ getCoroutineHandle(),
4114-
isUnwind ? Builder.getTrue() : Builder.getFalse(),
4140+
{ getCoroutineHandle(), Builder.getFalse(),
41154141
llvm::ConstantTokenNone::get(Builder.getContext())});
41164142

41174143
Builder.CreateUnreachable();
@@ -4311,7 +4337,7 @@ void IRGenSILFunction::visitThrowAddrInst(swift::ThrowAddrInst *i) {
43114337
}
43124338

43134339
void IRGenSILFunction::visitUnwindInst(swift::UnwindInst *i) {
4314-
// Just call coro.end marking unwind return
4340+
// Call coro.end marking unwind return
43154341
emitCoroutineOrAsyncExit(true);
43164342
}
43174343

‎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)