Skip to content

Commit debe0dd

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 a50079c commit debe0dd

8 files changed

+184
-71
lines changed

‎lib/IRGen/GenCall.cpp

+85-21
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

+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
@@ -4090,20 +4090,46 @@ void IRGenSILFunction::visitUnreachableInst(swift::UnreachableInst *i) {
40904090
}
40914091

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

4097-
// Emit the block.
4116+
return;
4117+
}
4118+
4119+
// Otherwise, create it and branch to it.
4120+
coroEndBB = createBasicBlock("coro.end");
4121+
setCoroutineExitBlock(coroEndBB);
4122+
Builder.CreateBr(coroEndBB);
40984123
Builder.emitBlock(coroEndBB);
4124+
40994125
if (isAsync())
41004126
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end_async,
4101-
{ getCoroutineHandle(),
4102-
isUnwind ? Builder.getTrue() : Builder.getFalse()});
4127+
{ getCoroutineHandle(), Builder.getFalse()});
41034128
else
4129+
// Do not bother about results here, normal result emission code would
4130+
// update token value.
41044131
Builder.CreateIntrinsicCall(llvm::Intrinsic::coro_end,
4105-
{ getCoroutineHandle(),
4106-
isUnwind ? Builder.getTrue() : Builder.getFalse(),
4132+
{ getCoroutineHandle(), Builder.getFalse(),
41074133
llvm::ConstantTokenNone::get(Builder.getContext())});
41084134

41094135
Builder.CreateUnreachable();
@@ -4303,7 +4329,7 @@ void IRGenSILFunction::visitThrowAddrInst(swift::ThrowAddrInst *i) {
43034329
}
43044330

43054331
void IRGenSILFunction::visitUnwindInst(swift::UnwindInst *i) {
4306-
// Just call coro.end marking unwind return
4332+
// Call coro.end marking unwind return
43074333
emitCoroutineOrAsyncExit(true);
43084334
}
43094335

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