Skip to content

Commit 57c0c78

Browse files
committed
Make Builtin.getCurrentExecutor conservatively not readnone.
Fixes a miscompile where the hop_to_executor optimizer would remove hop_to_executor before a getCurrentExecutor.
1 parent 061daa8 commit 57c0c78

File tree

3 files changed

+36
-1
lines changed

3 files changed

+36
-1
lines changed

include/swift/AST/Builtins.def

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,12 +758,15 @@ BUILTIN_MISC_OPERATION(BuildSerialExecutorRef,
758758
// Retrieve the ExecutorRef on which the current asynchronous
759759
// function is executing.
760760
// Does not retain an actor executor.
761-
BUILTIN_MISC_OPERATION_WITH_SILGEN(GetCurrentExecutor, "getCurrentExecutor", "n", Special)
761+
BUILTIN_MISC_OPERATION_WITH_SILGEN(GetCurrentExecutor, "getCurrentExecutor", "", Special)
762762

763763
// getCurrentAsyncTask: () -> Builtin.NativeObject
764764
//
765765
// Retrieve the pointer to the task in which the current asynchronous
766766
// function is executing.
767+
//
768+
// This is readnone because, within the world modeled by SIL, the
769+
// current async task of a thread never changes.
767770
BUILTIN_MISC_OPERATION_WITH_SILGEN(GetCurrentAsyncTask, "getCurrentAsyncTask", "n", Special)
768771

769772
/// cancelAsyncTask(): (Builtin.NativeObject) -> Void

lib/SIL/Utils/MemAccessUtils.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1857,6 +1857,7 @@ static void visitBuiltinAddress(BuiltinInst *builtin,
18571857
case BuiltinValueKind::AutoDiffAllocateSubcontext:
18581858
case BuiltinValueKind::InitializeDefaultActor:
18591859
case BuiltinValueKind::DestroyDefaultActor:
1860+
case BuiltinValueKind::GetCurrentExecutor:
18601861
return;
18611862

18621863
// General memory access to a pointer in first operand position.

test/SILOptimizer/optimize_hop_to_executor.sil

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,3 +208,34 @@ bb2:
208208
return %r : $()
209209
}
210210

211+
// This pattern is definitely optimizable, but it's *not* supposed to
212+
// be optimized by simply removing the hop before the builtin.
213+
// CHECK-LABEL: sil [ossa] @handleGetCurrentExecutor : $@convention(method) @async (@guaranteed MyActor, @guaranteed MyActor) -> () {
214+
// CHECK: bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor):
215+
// CHECK-NEXT: // function_ref
216+
// CHECK-NEXT: function_ref
217+
// CHECK-NEXT: apply
218+
// CHECK-NEXT: hop_to_executor
219+
// CHECK-NEXT: builtin
220+
// CHECK-NEXT: hop_to_executor
221+
// CHECK-NEXT: // function_ref
222+
// CHECK-NEXT: function_ref
223+
// CHECK-NEXT: apply
224+
// CHECK-NEXT: tuple
225+
// CHECK-NEXT: return
226+
// CHECK: } // end sil function 'handleGetCurrentExecutor'
227+
sil [ossa] @handleGetCurrentExecutor : $@convention(method) @async (@guaranteed MyActor, @guaranteed MyActor) -> () {
228+
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor):
229+
hop_to_executor %0 : $MyActor
230+
%async_f = function_ref @anotherAsyncFunction : $@convention(thin) @async () -> ()
231+
apply %async_f() : $@convention(thin) @async () -> ()
232+
hop_to_executor %0 : $MyActor
233+
%curExec = builtin "getCurrentExecutor"() : $Builtin.Executor
234+
hop_to_executor %1 : $MyActor
235+
%f = function_ref @requiredToRunOnActor : $@convention(method) (@guaranteed MyActor) -> ()
236+
apply %f(%1) : $@convention(method) (@guaranteed MyActor) -> ()
237+
hop_to_executor %0 : $MyActor
238+
%r = tuple ()
239+
return %r : $()
240+
}
241+

0 commit comments

Comments
 (0)