Skip to content

Commit 398466e

Browse files
rjmccallrjmccall
committed
Pass the task option record pointer to swift_task_create as a pointer.
We add the `memory(argmem: readwrite)` attribute to swift_task_create, which means that the call is only allowed to read or write "pointer operands". LLVM is smart enough to look through obvious ptrtoint casts, but not to look through integer selects and so on, which is what we produce when there's an opaque optional operand that feeds into the builtin. This was causing miscompiles under optimization when using `@isolated(any)` function types for task creation, since we're not yet clever enough to fold the function_extract_isolation for a known function (and of course it's not necessarily a known function anyway).
1 parent 0bc2198 commit 398466e

File tree

5 files changed

+21
-21
lines changed

5 files changed

+21
-21
lines changed

‎lib/IRGen/GenBuiltin.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
230230
auto taskOptions = args.claimNext();
231231
auto taskFunction = args.claimNext();
232232
auto taskContext = args.claimNext();
233+
taskOptions = IGF.Builder.CreateIntToPtr(taskOptions,
234+
IGF.IGM.SwiftTaskOptionRecordPtrTy);
233235

234236
auto asyncLet = emitBuiltinStartAsyncLet(
235237
IGF,
@@ -249,6 +251,8 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
249251
auto taskFunction = args.claimNext();
250252
auto taskContext = args.claimNext();
251253
auto localBuffer = args.claimNext();
254+
taskOptions = IGF.Builder.CreateIntToPtr(taskOptions,
255+
IGF.IGM.SwiftTaskOptionRecordPtrTy);
252256

253257
auto asyncLet = emitBuiltinStartAsyncLet(
254258
IGF,

‎lib/IRGen/GenConcurrency.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ irgen::emitTaskCreate(IRGenFunction &IGF, llvm::Value *flags,
705705
Explosion &taskFunction,
706706
SubstitutionMap subs) {
707707
llvm::Value *taskOptions =
708-
llvm::ConstantInt::get(IGF.IGM.SwiftTaskOptionRecordPtrTy, 0);
708+
llvm::ConstantPointerNull::get(IGF.IGM.SwiftTaskOptionRecordPtrTy);
709709

710710
CanType resultType;
711711
if (subs) {

‎lib/IRGen/IRGenModule.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -675,13 +675,14 @@ IRGenModule::IRGenModule(IRGenerator &irgen,
675675
AsyncFunctionPointerPtrTy = AsyncFunctionPointerTy->getPointerTo(DefaultAS);
676676
SwiftTaskPtrTy = SwiftTaskTy->getPointerTo(DefaultAS);
677677
SwiftAsyncLetPtrTy = Int8PtrTy; // we pass it opaquely (AsyncLet*)
678-
SwiftTaskOptionRecordPtrTy = SizeTy; // Builtin.RawPointer? that we get as (TaskOptionRecord*)
679-
SwiftTaskGroupPtrTy = Int8PtrTy; // we pass it opaquely (TaskGroup*)
680-
SwiftTaskOptionRecordTy = createStructType(
681-
*this, "swift.task_option", {
678+
SwiftTaskOptionRecordTy =
679+
llvm::StructType::create(getLLVMContext(), "swift.task_option");
680+
SwiftTaskOptionRecordPtrTy = SwiftTaskOptionRecordTy->getPointerTo(DefaultAS);
681+
SwiftTaskOptionRecordTy->setBody({
682682
SizeTy, // Flags
683683
SwiftTaskOptionRecordPtrTy, // Parent
684684
});
685+
SwiftTaskGroupPtrTy = Int8PtrTy; // we pass it opaquely (TaskGroup*)
685686
SwiftTaskGroupTaskOptionRecordTy = createStructType(
686687
*this, "swift.task_group_task_option", {
687688
SwiftTaskOptionRecordTy, // Base option record

‎lib/IRGen/IRGenModule.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -816,7 +816,7 @@ class IRGenModule {
816816
llvm::PointerType *SwiftContextPtrTy;
817817
llvm::PointerType *SwiftTaskPtrTy;
818818
llvm::PointerType *SwiftAsyncLetPtrTy;
819-
llvm::IntegerType *SwiftTaskOptionRecordPtrTy;
819+
llvm::PointerType *SwiftTaskOptionRecordPtrTy;
820820
llvm::PointerType *SwiftTaskGroupPtrTy;
821821
llvm::StructType *SwiftTaskOptionRecordTy;
822822
llvm::StructType *SwiftInitialSerialExecutorTaskOptionRecordTy;

‎test/IRGen/async/builtins.sil

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ bb0(%0 : @guaranteed $Builtin.NativeObject):
3333
sil hidden [ossa] @launch_future : $@convention(method) <T> (Int, @owned @Sendable @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>) -> () {
3434
bb0(%flags : $Int, %taskFunction: @owned $@Sendable @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <T>):
3535
// CHECK-NOT: br i1
36-
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %0, [[INT]] 0, ptr %T, ptr %1, ptr %2)
36+
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %0, ptr null, ptr %T, ptr %1, ptr %2)
3737
%optSerialExecutor = enum $Optional<Builtin.Executor>, #Optional.none
3838
%optTaskGroup = enum $Optional<Builtin.RawPointer>, #Optional.none
3939
%optTaskExecutor = enum $Optional<Builtin.Executor>, #Optional.none
@@ -52,14 +52,13 @@ bb0(%flags : $Int, %serialExecutor : $Builtin.Executor, %taskFunction: @owned $@
5252
// CHECK: [[FLAGS_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 0
5353
// CHECK: store [[INT]] 0, ptr [[FLAGS_GEP]], align
5454
// CHECK: [[PARENT_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 1
55-
// CHECK: store [[INT]] 0, ptr [[PARENT_GEP]], align
55+
// CHECK: store ptr null, ptr [[PARENT_GEP]], align
5656
// CHECK: [[EXECUTOR_GEP:%.*]] = getelementptr inbounds %swift.serial_executor_task_option, ptr [[EXECUTOR_RECORD]], i32 0, i32 1
5757
// CHECK: [[EXECUTOR_IDENT_GEP:%.*]] = getelementptr inbounds %swift.executor, ptr [[EXECUTOR_GEP]], i32 0, i32 0
5858
// CHECK: store [[INT]] %1, ptr [[EXECUTOR_IDENT_GEP]], align
5959
// CHECK: [[EXECUTOR_IMPL_GEP:%.*]] = getelementptr inbounds %swift.executor, ptr [[EXECUTOR_GEP]], i32 0, i32 1
6060
// CHECK: store [[INT]] %2, ptr [[EXECUTOR_IMPL_GEP]], align
61-
// CHECK: [[OPTIONS_PTR:%.*]] = ptrtoint ptr [[EXECUTOR_RECORD]] to [[INT]]
62-
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %0, [[INT]] [[OPTIONS_PTR]], ptr %T, ptr %3, ptr %4)
61+
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %0, ptr [[EXECUTOR_RECORD]], ptr %T, ptr %3, ptr %4)
6362
%optSerialExecutor = enum $Optional<Builtin.Executor>, #Optional.some, %serialExecutor : $Builtin.Executor
6463
%optTaskGroup = enum $Optional<Builtin.RawPointer>, #Optional.none
6564
%optTaskExecutor = enum $Optional<Builtin.Executor>, #Optional.none
@@ -76,11 +75,10 @@ bb0(%flags : $Int, %serialExecutor : $Builtin.Executor, %taskFunction: @owned $@
7675
// CHECK: [[FLAGS_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 0
7776
// CHECK: store [[INT]] 1, ptr [[FLAGS_GEP]], align
7877
// CHECK: [[PARENT_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 1
79-
// CHECK: store [[INT]] 0, ptr [[PARENT_GEP]], align
78+
// CHECK: store ptr null, ptr [[PARENT_GEP]], align
8079
// CHECK: [[GROUP_GEP:%.*]] = getelementptr inbounds %swift.task_group_task_option, ptr [[OPTIONS]], i32 0, i32 1
8180
// CHECK: store ptr %0, ptr [[GROUP_GEP]], align
82-
// CHECK: [[OPTIONS_PTR:%.*]] = ptrtoint ptr [[OPTIONS]] to [[INT]]
83-
// CHECK: call swiftcc %swift.async_task_and_context @swift_task_create([[INT]] %3, [[INT]] [[OPTIONS_PTR]],
81+
// CHECK: call swiftcc %swift.async_task_and_context @swift_task_create([[INT]] %3, ptr [[OPTIONS]],
8482
sil hidden @launch_future_in_group : $@convention(thin) (Builtin.RawPointer, @owned @Sendable @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <Int>, Int) -> () {
8583
bb0(%taskGroup : $Builtin.RawPointer, %taskFunction : $@Sendable @async @callee_guaranteed @substituted <τ_0_0> () -> (@out τ_0_0, @error Error) for <Int>, %flags: $Int):
8684
%optSerialExecutor = enum $Optional<Builtin.Executor>, #Optional.none
@@ -105,13 +103,12 @@ bb0(%flags : $Int, %optTaskGroup : $Optional<Builtin.RawPointer>, %taskFunction:
105103
// CHECK: [[FLAGS_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 0
106104
// CHECK: store [[INT]] 1, ptr [[FLAGS_GEP]], align
107105
// CHECK: [[PARENT_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 1
108-
// CHECK: store [[INT]] 0, ptr [[PARENT_GEP]], align
106+
// CHECK: store ptr null, ptr [[PARENT_GEP]], align
109107
// CHECK: [[GROUP_GEP:%.*]] = getelementptr inbounds %swift.task_group_task_option, ptr [[OPTIONS]], i32 0, i32 1
110108
// CHECK: store ptr [[GROUP]], ptr [[GROUP_GEP]], align
111-
// CHECK: [[OPTIONS_PTR:%.*]] = ptrtoint ptr [[OPTIONS]] to [[INT]]
112109
// CHECK: br label %task_group.cont
113110
// CHECK: task_group.cont:
114-
// CHECK: phi [[INT]] [ 0, %entry ], [ [[OPTIONS_PTR]], %task_group.some ]
111+
// CHECK: phi ptr [ null, %entry ], [ [[OPTIONS]], %task_group.some ]
115112
// CHECK: [[NEW_TASK_AND_CONTEXT:%.*]] = call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create(
116113

117114
%optSerialExecutor = enum $Optional<Builtin.Executor>, #Optional.none
@@ -151,24 +148,22 @@ bb0(%taskGroup : $Builtin.RawPointer, %taskExecutor : $Builtin.Executor, %taskFu
151148
// CHECK: [[FLAGS_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 0
152149
// CHECK: store [[INT]] 1, ptr [[FLAGS_GEP]], align
153150
// CHECK: [[PARENT_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 1
154-
// CHECK: store [[INT]] 0, ptr [[PARENT_GEP]], align
151+
// CHECK: store ptr null, ptr [[PARENT_GEP]], align
155152
// CHECK: [[GROUP_GEP:%.*]] = getelementptr inbounds %swift.task_group_task_option, ptr [[GROUP_RECORD]], i32 0, i32 1
156153
// CHECK: store ptr %0, ptr [[GROUP_GEP]], align
157-
// CHECK: [[OPTIONS_PTR:%.*]] = ptrtoint ptr [[GROUP_RECORD]] to [[INT]]
158154

159155
// CHECK: [[BASE_GEP:%.*]] = getelementptr inbounds %swift.task_executor_task_option, ptr [[EXECUTOR_RECORD]], i32 0, i32 0
160156
// CHECK: [[FLAGS_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 0
161157
// CHECK: store [[INT]] 5, ptr [[FLAGS_GEP]], align
162158
// CHECK: [[PARENT_GEP:%.*]] = getelementptr inbounds %swift.task_option, ptr [[BASE_GEP]], i32 0, i32 1
163-
// CHECK: store [[INT]] [[OPTIONS_PTR]], ptr [[PARENT_GEP]], align
159+
// CHECK: store ptr [[GROUP_RECORD]], ptr [[PARENT_GEP]], align
164160
// CHECK: [[EXECUTOR_GEP:%.*]] = getelementptr inbounds %swift.task_executor_task_option, ptr [[EXECUTOR_RECORD]], i32 0, i32 1
165161
// CHECK: [[EXECUTOR_IDENT_GEP:%.*]] = getelementptr inbounds %swift.executor, ptr [[EXECUTOR_GEP]], i32 0, i32 0
166162
// CHECK: store [[INT]] %1, ptr [[EXECUTOR_IDENT_GEP]], align
167163
// CHECK: [[EXECUTOR_IMPL_GEP:%.*]] = getelementptr inbounds %swift.executor, ptr [[EXECUTOR_GEP]], i32 0, i32 1
168164
// CHECK: store [[INT]] %2, ptr [[EXECUTOR_IMPL_GEP]], align
169-
// CHECK: [[OPTIONS_PTR:%.*]] = ptrtoint ptr [[EXECUTOR_RECORD]] to [[INT]]
170165

171-
// CHECK: call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %5, [[INT]] [[OPTIONS_PTR]]
166+
// CHECK: call swift{{(tail)?}}cc %swift.async_task_and_context @swift_task_create([[INT]] %5, ptr [[EXECUTOR_RECORD]]
172167
%9 = builtin "createAsyncTask"(%flags : $Int, %optSerialExecutor : $Optional<Builtin.Executor>, %optTaskGroup : $Optional<Builtin.RawPointer>, %optTaskExecutor: $Optional<Builtin.Executor>, %taskFunction : $@Sendable @async @callee_guaranteed () -> @error Error) : $(Builtin.NativeObject, Builtin.RawPointer)
173168
%10 = tuple_extract %9 : $(Builtin.NativeObject, Builtin.RawPointer), 0
174169
strong_release %10 : $Builtin.NativeObject

0 commit comments

Comments
 (0)