Skip to content

Commit 511f9aa

Browse files
jckarterjckarter
committed
SIL: Clean up ownership handling in createAsyncTask builtins.
The underlying runtime functions really want to be able to consume the closure being used to spawn the task, but the implementation was trying to hide this by introducing a retain at IRGen time, which is not very ARC-optimizer-friendly. Correctly model the builtin operands as consumed so that the ownership verifier allows them to be taken +1.
1 parent 2c85502 commit 511f9aa

File tree

5 files changed

+82
-31
lines changed

5 files changed

+82
-31
lines changed

‎lib/IRGen/GenBuiltin.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -244,11 +244,6 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
244244
auto taskFunction = args.claimNext();
245245
auto taskContext = args.claimNext();
246246

247-
// FIXME: SIL treats the function/context parameter as "guaranteed", but
248-
// the runtime entry point assumes it is owned. Introduce an extra retain
249-
// of the context to balance things out.
250-
IGF.emitNativeStrongRetain(taskContext, IGF.getDefaultAtomicity());
251-
252247
auto newTaskAndContext = emitTaskCreate(
253248
IGF, flags, parentTask, taskGroup, futureResultType, taskFunction, taskContext,
254249
substitutions);

‎lib/SIL/IR/OperandOwnership.cpp

Lines changed: 46 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -574,6 +574,9 @@ namespace {
574574
struct OperandOwnershipBuiltinClassifier
575575
: SILBuiltinVisitor<OperandOwnershipBuiltinClassifier, OperandOwnership> {
576576
using Map = OperandOwnership;
577+
578+
const Operand &op;
579+
OperandOwnershipBuiltinClassifier(const Operand &op) : op(op) {}
577580

578581
OperandOwnership visitLLVMIntrinsic(BuiltinInst *bi, llvm::Intrinsic::ID id) {
579582
// LLVM intrinsics do not traffic in ownership, so if we have a result, it
@@ -744,14 +747,49 @@ BUILTIN_OPERAND_OWNERSHIP(InstantaneousUse, IntInstrprofIncrement)
744747
BUILTIN_OPERAND_OWNERSHIP(ForwardingConsume, COWBufferForReading)
745748
BUILTIN_OPERAND_OWNERSHIP(ForwardingConsume, UnsafeGuaranteed)
746749

747-
// FIXME: These are considered InteriorPointer because they may propagate a
748-
// pointer into a borrowed values. If they do not propagate an interior pointer,
749-
// then they should be InstantaneousUse instead and should not require a
750-
// guaranteed value.
750+
OperandOwnership
751+
OperandOwnershipBuiltinClassifier::visitCreateAsyncTask(BuiltinInst *bi,
752+
StringRef attr) {
753+
// The function operand is consumed by the new task.
754+
if (&op == &bi->getOperandRef(2))
755+
return OperandOwnership::DestroyingConsume;
756+
757+
// FIXME: These are considered InteriorPointer because they may propagate a
758+
// pointer into a borrowed values. If they do not propagate an interior pointer,
759+
// then they should be InstantaneousUse instead and should not require a
760+
// guaranteed value.
761+
return OperandOwnership::InteriorPointer;
762+
}
763+
764+
OperandOwnership
765+
OperandOwnershipBuiltinClassifier::visitCreateAsyncTaskFuture(BuiltinInst *bi,
766+
StringRef attr) {
767+
// The function operand is consumed by the new task.
768+
if (&op == &bi->getOperandRef(3))
769+
return OperandOwnership::DestroyingConsume;
770+
771+
// FIXME: These are considered InteriorPointer because they may propagate a
772+
// pointer into a borrowed values. If they do not propagate an interior pointer,
773+
// then they should be InstantaneousUse instead and should not require a
774+
// guaranteed value.
775+
return OperandOwnership::InteriorPointer;
776+
}
777+
778+
OperandOwnership
779+
OperandOwnershipBuiltinClassifier::visitCreateAsyncTaskGroupFuture(BuiltinInst *bi,
780+
StringRef attr) {
781+
// The function operand is consumed by the new task.
782+
if (&op == &bi->getOperandRef(4))
783+
return OperandOwnership::DestroyingConsume;
784+
785+
// FIXME: These are considered InteriorPointer because they may propagate a
786+
// pointer into a borrowed values. If they do not propagate an interior pointer,
787+
// then they should be InstantaneousUse instead and should not require a
788+
// guaranteed value.
789+
return OperandOwnership::InteriorPointer;
790+
}
791+
751792
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CancelAsyncTask)
752-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTask)
753-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTaskFuture)
754-
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, CreateAsyncTaskGroupFuture)
755793
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, InitializeDefaultActor)
756794
BUILTIN_OPERAND_OWNERSHIP(InteriorPointer, DestroyDefaultActor)
757795

@@ -787,7 +825,7 @@ SHOULD_NEVER_VISIT_BUILTIN(GetCurrentAsyncTask)
787825
#include "swift/AST/Builtins.def"
788826

789827
OperandOwnership OperandOwnershipClassifier::visitBuiltinInst(BuiltinInst *bi) {
790-
return OperandOwnershipBuiltinClassifier().check(bi);
828+
return OperandOwnershipBuiltinClassifier(op).check(bi);
791829
}
792830

793831
//===----------------------------------------------------------------------===//

‎lib/SILGen/SILGenBuiltin.cpp

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,19 +1377,32 @@ static ManagedValue emitBuiltinCancelAsyncTask(
13771377
return SGF.emitCancelAsyncTask(loc, args[0].borrow(SGF, loc).forward(SGF));
13781378
}
13791379

1380+
// Helper to lower a function argument to be usable as the entry point of a
1381+
// new async task
1382+
static ManagedValue
1383+
emitFunctionArgumentForAsyncTaskEntryPoint(SILGenFunction &SGF,
1384+
SILLocation loc,
1385+
ManagedValue function,
1386+
CanType formalReturnTy) {
1387+
// The function is consumed by the underlying runtime call.
1388+
return function.ensurePlusOne(SGF, loc);
1389+
}
1390+
13801391
// Emit SIL for the named builtin: createAsyncTask.
13811392
static ManagedValue emitBuiltinCreateAsyncTask(
13821393
SILGenFunction &SGF, SILLocation loc, SubstitutionMap subs,
13831394
ArrayRef<ManagedValue> args, SGFContext C) {
13841395
ASTContext &ctx = SGF.getASTContext();
13851396
auto flags = args[0].forward(SGF);
13861397
auto parentTask = args[1].borrow(SGF, loc).forward(SGF);
1387-
auto function = args[2].borrow(SGF, loc).forward(SGF);
1398+
auto function = emitFunctionArgumentForAsyncTaskEntryPoint(SGF, loc,
1399+
args[2], TupleType::getEmpty(SGF.getASTContext()));
1400+
13881401
auto apply = SGF.B.createBuiltin(
13891402
loc,
13901403
ctx.getIdentifier(getBuiltinName(BuiltinValueKind::CreateAsyncTask)),
13911404
SGF.getLoweredType(getAsyncTaskAndContextType(ctx)), subs,
1392-
{ flags, parentTask, function });
1405+
{ flags, parentTask, function.forward(SGF) });
13931406
return SGF.emitManagedRValueWithCleanup(apply);
13941407
}
13951408

@@ -1417,13 +1430,14 @@ static ManagedValue emitBuiltinCreateAsyncTaskFuture(
14171430
SGF.B.createMetatype(loc, SGF.getLoweredType(futureResultType)));
14181431
}).borrow(SGF, loc).forward(SGF);
14191432

1420-
auto function = args[2].borrow(SGF, loc).forward(SGF);
1433+
auto function = emitFunctionArgumentForAsyncTaskEntryPoint(SGF, loc, args[2],
1434+
futureResultType);
14211435
auto apply = SGF.B.createBuiltin(
14221436
loc,
14231437
ctx.getIdentifier(
14241438
getBuiltinName(BuiltinValueKind::CreateAsyncTaskFuture)),
14251439
SGF.getLoweredType(getAsyncTaskAndContextType(ctx)), subs,
1426-
{ flags, parentTask, futureResultMetadata, function });
1440+
{ flags, parentTask, futureResultMetadata, function.forward(SGF) });
14271441
return SGF.emitManagedRValueWithCleanup(apply);
14281442
}
14291443

@@ -1452,13 +1466,14 @@ static ManagedValue emitBuiltinCreateAsyncTaskGroupFuture(
14521466
SGF.B.createMetatype(loc, SGF.getLoweredType(futureResultType)));
14531467
}).borrow(SGF, loc).forward(SGF);
14541468

1455-
auto function = args[3].borrow(SGF, loc).forward(SGF);
1469+
auto function = emitFunctionArgumentForAsyncTaskEntryPoint(SGF, loc, args[3],
1470+
futureResultType);
14561471
auto apply = SGF.B.createBuiltin(
14571472
loc,
14581473
ctx.getIdentifier(
14591474
getBuiltinName(BuiltinValueKind::CreateAsyncTaskGroupFuture)),
14601475
SGF.getLoweredType(getAsyncTaskAndContextType(ctx)), subs,
1461-
{ flags, parentTask, group, futureResultMetadata, function });
1476+
{ flags, parentTask, group, futureResultMetadata, function.forward(SGF) });
14621477
return SGF.emitManagedRValueWithCleanup(apply);
14631478
}
14641479

‎test/IRGen/async/builtins.sil

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,18 +33,19 @@ bb0(%0 : @guaranteed $Builtin.NativeObject):
3333
}
3434

3535
// CHECK-LABEL: define hidden swift{{(tail)?}}cc void @launch_task
36-
sil hidden [ossa] @launch_task : $@convention(method) @async (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_guaranteed () -> (@error Error)) -> () {
37-
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_guaranteed () -> (@error Error)):
36+
sil hidden [ossa] @launch_task : $@convention(method) @async (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_owned () -> (@error Error)) -> () {
37+
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_owned () -> (@error Error)):
3838
%copy = copy_value %1 : $Optional<Builtin.NativeObject>
3939
%3 = begin_borrow %copy : $Optional<Builtin.NativeObject>
40+
%4 = copy_value %2 : $@async @callee_owned () -> (@error Error)
4041
// CHECK: [[FN_CONTEXT:%.*]] = load %swift.refcounted*, %swift.refcounted** %.data
4142
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[FN_CONTEXT]])
4243
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create_f(
4344
// CHECK-NEXT: [[NEW_CONTEXT_RAW:%.*]] = extractvalue %swift.async_task_and_context [[NEW_TASK_AND_CONTEXT]], 1
4445
// CHECK-NEXT: [[NEW_CONTEXT:%.*]] = bitcast %swift.context* [[NEW_CONTEXT_RAW]] to
4546
// CHECK-NEXT: [[CONTEXT_INFO_LOC:%.*]] = getelementptr inbounds <{{.*}}>* [[NEW_CONTEXT]]
4647
// CHECK-NEXT: store %swift.refcounted* [[FN_CONTEXT]], %swift.refcounted** [[CONTEXT_INFO_LOC]]
47-
%20 = builtin "createAsyncTask"(%0 : $Int, %3 : $Optional<Builtin.NativeObject>, %2 : $@async @callee_guaranteed () -> (@error Error)) : $(Builtin.NativeObject, Builtin.RawPointer)
48+
%20 = builtin "createAsyncTask"(%0 : $Int, %3 : $Optional<Builtin.NativeObject>, %4 : $@async @callee_owned () -> (@error Error)) : $(Builtin.NativeObject, Builtin.RawPointer)
4849
end_borrow %3 : $Optional<Builtin.NativeObject>
4950
destroy_value %copy : $Optional<Builtin.NativeObject>
5051
destroy_value %20 : $(Builtin.NativeObject, Builtin.RawPointer)
@@ -53,12 +54,13 @@ bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@a
5354
}
5455

5556
// CHECK-LABEL: define hidden swift{{(tail)?}}cc void @launch_future
56-
sil hidden [ossa] @launch_future : $@convention(method) <T> (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>, @in_guaranteed T) -> () {
57-
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>, %3: $*T):
57+
sil hidden [ossa] @launch_future : $@convention(method) <T> (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>, @in_guaranteed T) -> () {
58+
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>, %3: $*T):
5859
%copy = copy_value %1 : $Optional<Builtin.NativeObject>
5960
// CHECK-32: [[TEMP:%.*]] = inttoptr i32 %1 to %swift.refcounted*
6061
// CHECK-64: [[TEMP:%.*]] = inttoptr i64 %1 to %swift.refcounted*
6162
%4 = begin_borrow %copy : $Optional<Builtin.NativeObject>
63+
%5 = copy_value %2 : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>
6264
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[TEMP]])
6365
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[FN_CONTEXT:%.*]])
6466
%9 = metatype $@thick T.Type
@@ -69,7 +71,7 @@ bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@a
6971
// CHECK-32-NEXT: [[CONTEXT_INFO_LOC:%.*]] = getelementptr inbounds <{{.*}}>* [[NEW_CONTEXT]], i32 0, i32 6
7072
// CHECK-64-NEXT: [[CONTEXT_INFO_LOC:%.*]] = getelementptr inbounds <{{.*}}>* [[NEW_CONTEXT]], i32 0, i32 7
7173
// CHECK-NEXT: store %swift.refcounted* [[FN_CONTEXT]], %swift.refcounted** [[CONTEXT_INFO_LOC]]
72-
%20 = builtin "createAsyncTaskFuture"<T>(%0 : $Int, %4 : $Optional<Builtin.NativeObject>, %10 : $@thick Any.Type, %2 : $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
74+
%20 = builtin "createAsyncTaskFuture"<T>(%0 : $Int, %4 : $Optional<Builtin.NativeObject>, %10 : $@thick Any.Type, %5 : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
7375
end_borrow %4 : $Optional<Builtin.NativeObject>
7476
destroy_value %copy : $Optional<Builtin.NativeObject>
7577
destroy_value %20 : $(Builtin.NativeObject, Builtin.RawPointer)
@@ -78,10 +80,11 @@ bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@a
7880
}
7981

8082
// CHECK-LABEL: define hidden swift{{(tail)?}}cc void @launch_void_future
81-
sil hidden [ossa] @launch_void_future : $@convention(method) (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>) -> () {
82-
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>):
83+
sil hidden [ossa] @launch_void_future : $@convention(method) (Int, Optional<Builtin.NativeObject>, @guaranteed @async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>) -> () {
84+
bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>):
8385
%copy = copy_value %1 : $Optional<Builtin.NativeObject>
8486
%4 = begin_borrow %copy : $Optional<Builtin.NativeObject>
87+
%5 = copy_value %2 : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>
8588
// CHECK-32: [[TEMP:%.*]] = inttoptr i32 %1 to %swift.refcounted*
8689
// CHECK-64: [[TEMP:%.*]] = inttoptr i64 %1 to %swift.refcounted*
8790
// CHECK: call %swift.refcounted* @swift_retain(%swift.refcounted* returned [[TEMP]])
@@ -94,7 +97,7 @@ bb0(%0 : $Int, %1: @unowned $Optional<Builtin.NativeObject>, %2: @guaranteed $@a
9497
// CHECK-32-NEXT: [[CONTEXT_INFO_LOC:%.*]] = getelementptr inbounds <{{.*}}>* [[NEW_CONTEXT]], i32 0, i32 6
9598
// CHECK-64-NEXT: [[CONTEXT_INFO_LOC:%.*]] = getelementptr inbounds <{{.*}}>* [[NEW_CONTEXT]], i32 0, i32 7
9699
// CHECK-NEXT: store %swift.refcounted* [[FN_CONTEXT]], %swift.refcounted** [[CONTEXT_INFO_LOC]]
97-
%20 = builtin "createAsyncTaskFuture"<()>(%0 : $Int, %4 : $Optional<Builtin.NativeObject>, %9 : $@thick Any.Type, %2 : $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>) : $(Builtin.NativeObject, Builtin.RawPointer)
100+
%20 = builtin "createAsyncTaskFuture"<()>(%0 : $Int, %4 : $Optional<Builtin.NativeObject>, %9 : $@thick Any.Type, %5 : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <()>) : $(Builtin.NativeObject, Builtin.RawPointer)
98101
end_borrow %4 : $Optional<Builtin.NativeObject>
99102
destroy_value %copy : $Optional<Builtin.NativeObject>
100103
destroy_value %20 : $(Builtin.NativeObject, Builtin.RawPointer)

‎test/SILGen/async_builtins.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public struct X {
1818

1919
// CHECK-LABEL: sil hidden [ossa] @$s4test1XV10launchTaskyyYF : $@convention(method) @async (X) -> ()
2020
func launchTask() async {
21-
// CHECK: builtin "createAsyncTask"([[FLAGS:%.*]] : $Int, [[PARENT:%.*]] : $Optional<Builtin.NativeObject>, [[FN:%.*]] : $@async @callee_guaranteed () -> @error Error) : $(Builtin.NativeObject, Builtin.RawPointer)
21+
// CHECK: builtin "createAsyncTask"([[FLAGS:%.*]] : $Int, [[PARENT:%.*]] : $Optional<Builtin.NativeObject>, [[FN:%.*]] : $@async @callee_owned () -> @error Error) : $(Builtin.NativeObject, Builtin.RawPointer)
2222
let task = Builtin.getCurrentAsyncTask()
2323
let childTask = Builtin.createAsyncTask(0, task) {
2424
await launchTask()
@@ -28,15 +28,15 @@ public struct X {
2828

2929
// CHECK-LABEL: sil hidden [ossa] @$s4test1XV12launchFutureyyxlF : $@convention(method) <T> (@in_guaranteed T, X) -> ()
3030
func launchFuture<T>(_ value: T) {
31-
// CHECK: builtin "createAsyncTaskFuture"<T>([[ZERO:%.*]] : $Int, [[NIL:%.*]] : $Optional<Builtin.NativeObject>, [[FN:%.*]] : $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
31+
// CHECK: builtin "createAsyncTaskFuture"<T>([[ZERO:%.*]] : $Int, [[NIL:%.*]] : $Optional<Builtin.NativeObject>, [[FN:%.*]] : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
3232
let task = Builtin.createAsyncTaskFuture(0, nil) { () async throws -> T in
3333
return value
3434
}
3535
}
3636

3737
// CHECK-LABEL: sil hidden [ossa] @$s4test1XV16launchGroupChildyyxlF : $@convention(method) <T> (@in_guaranteed T, X) -> () {
3838
func launchGroupChild<T>(_ value: T) {
39-
// CHECK: builtin "createAsyncTaskGroupFuture"<T>([[ZERO:%.*]] : $Int, [[NIL:%.*]] : $Optional<Builtin.NativeObject>, [[NIL:%.*]] : $Optional<Builtin.RawPointer>, [[FN:%.*]] : $@async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
39+
// CHECK: builtin "createAsyncTaskGroupFuture"<T>([[ZERO:%.*]] : $Int, [[NIL:%.*]] : $Optional<Builtin.NativeObject>, [[NIL:%.*]] : $Optional<Builtin.RawPointer>, [[FN:%.*]] : $@async @callee_owned @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) : $(Builtin.NativeObject, Builtin.RawPointer)
4040
let task = Builtin.createAsyncTaskGroupFuture(0, nil, nil) { () async throws -> T in
4141
return value
4242
}

0 commit comments

Comments
 (0)