Skip to content

[concurrency] Fix optimize_hop_to_executor so that we take advantage of the new nonisolated(nonsending) ABI in post 6.2. #83295

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+49 −50
Open

[concurrency] Fix optimize_hop_to_executor so that we take advantage of the new nonisolated(nonsending) ABI in post 6.2. #83295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 45 additions & 40 deletions lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,12 @@ class OptimizeHopToExecutor {
/// Search for hop_to_executor instructions and add their operands to \p actors.
void OptimizeHopToExecutor::collectActors(Actors &actors) {
int uniqueActorID = 0;

if (auto isolation = function->getActorIsolation();
isolation && isolation->isCallerIsolationInheriting()) {
actors[function->maybeGetIsolatedArgument()] = uniqueActorID++;
}

for (SILBasicBlock &block : *function) {
for (SILInstruction &inst : block) {
if (auto *hop = dyn_cast<HopToExecutorInst>(&inst)) {
Expand Down Expand Up @@ -193,10 +199,7 @@ void OptimizeHopToExecutor::solveDataflowBackward() {
/// Returns true if \p inst is a suspension point or an async call.
static bool isSuspensionPoint(SILInstruction *inst) {
if (auto applySite = FullApplySite::isa(inst)) {
// NOTE: For 6.2, we consider nonisolated(nonsending) to be a suspension
// point, when it really isn't. We do this so that we have a truly
// conservative change that does not change output.
if (applySite.isAsync())
if (applySite.isAsync() && !applySite.isCallerIsolationInheriting())
return true;
return false;
}
Expand All @@ -213,8 +216,20 @@ bool OptimizeHopToExecutor::removeRedundantHopToExecutors(const Actors &actors)

// Initialize the dataflow.
for (BlockState &state : blockStates) {
state.entry = (state.block == function->getEntryBlock() ?
BlockState::Unknown : BlockState::NotSet);
state.entry = [&]() -> int {
if (state.block != function->getEntryBlock()) {
return BlockState::NotSet;
}

if (auto isolation = function->getActorIsolation();
isolation && isolation->isCallerIsolationInheriting()) {
auto *fArg =
cast<SILFunctionArgument>(function->maybeGetIsolatedArgument());
return actors.lookup(SILValue(fArg));
}

return BlockState::Unknown;
}();
state.intra = BlockState::NotSet;
for (SILInstruction &inst : *state.block) {
if (isSuspensionPoint(&inst)) {
Expand Down Expand Up @@ -316,44 +331,11 @@ void OptimizeHopToExecutor::updateNeedExecutor(int &needExecutor,
return;
}

// For 6.2 to be conservative, if we are calling a function with
// caller_isolation_inheriting isolation, treat the callsite as if the
// callsite is an instruction that needs an executor.
//
// DISCUSSION: The reason why we are doing this is that in 6.2, we are going
// to continue treating caller isolation inheriting functions as a suspension
// point for the purpose of eliminating redundant hop to executor to not make
// this optimization more aggressive. Post 6.2, we will stop treating caller
// isolation inheriting functions as suspension points, meaning this code can
// be deleted.
if (auto fas = FullApplySite::isa(inst);
fas && fas.isAsync() && fas.isCallerIsolationInheriting()) {
needExecutor = BlockState::ExecutorNeeded;
return;
}

// For 6.2, if we are in a caller isolation inheriting function, we need to
// treat its return as an executor needing function before
// isSuspensionPoint.
//
// DISCUSSION: We need to do this here since for 6.2, a caller isolation
// inheriting function is going to be considered a suspension point to be
// conservative and make this optimization strictly more conservative. Post
// 6.2, since caller isolation inheriting functions will no longer be
// considered suspension points, we will be able to sink this code into needs
// executor.
if (isa<ReturnInst>(inst)) {
if (auto isolation = inst->getFunction()->getActorIsolation();
isolation && isolation->isCallerIsolationInheriting()) {
needExecutor = BlockState::ExecutorNeeded;
return;
}
}

if (isSuspensionPoint(inst)) {
needExecutor = BlockState::NoExecutorNeeded;
return;
}

if (needsExecutor(inst))
needExecutor = BlockState::ExecutorNeeded;
}
Expand Down Expand Up @@ -403,6 +385,29 @@ bool OptimizeHopToExecutor::needsExecutor(SILInstruction *inst) {
if (isa<BeginBorrowInst>(inst) || isa<EndBorrowInst>(inst)) {
return false;
}

// A call to a caller isolation inheriting function does not create dead
// executors since caller isolation inheriting functions do not hop in their
// prologue.
if (auto fas = FullApplySite::isa(inst);
fas && fas.isAsync() && fas.isCallerIsolationInheriting()) {
return true;
}

// Treat returns from a caller isolation inheriting function as requiring the
// liveness of hop to executors before it.
//
// DISCUSSION: We do this since callers of callee functions with isolation
// inheriting isolation are not required to have a hop after the return from
// the callee function... so we have no guarantee that there isn't code in the
// caller that needs this hop to executor to run on the correct actor.
if (isa<ReturnInst>(inst)) {
if (auto isolation = inst->getFunction()->getActorIsolation();
isolation && isolation->isCallerIsolationInheriting()) {
return true;
}
}

return inst->mayReadOrWriteMemory();
}

Expand Down
4 changes: 0 additions & 4 deletions test/Concurrency/nonisolated_nonsending_optimize_hoptoexecutor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,11 @@
// REQUIRES: concurrency

// CHECK-LABEL: sil hidden [noinline] @$s4testAAyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
// CHECK: hop_to_executor
// CHECK: } // end sil function '$s4testAAyyYaF'
@inline(never)
nonisolated(nonsending) func test() async {}

// CHECK-LABEL: sil hidden [noinline] @$s4test5test2yyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
// CHECK: hop_to_executor
// CHECK: hop_to_executor
// CHECK: } // end sil function '$s4test5test2yyYaF'
@inline(never)
nonisolated(nonsending) func test2() async {
Expand All @@ -24,7 +21,6 @@ func test3() async {
// CHECK-LABEL: sil @$s4test6calleryyYaF : $@convention(thin) @async () -> () {
// CHECK: hop_to_executor
// CHECK: function_ref @$s4testAAyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: hop_to_executor
// CHECK: function_ref @$s4test5test2yyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> ()
// CHECK: } // end sil function '$s4test6calleryyYaF'
public func caller() async {
Expand Down
10 changes: 4 additions & 6 deletions test/SILOptimizer/optimize_hop_to_executor.sil
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,8 +393,7 @@ bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyAc
// CHECK-LABEL: sil [ossa] @callerIsolationInheritingStopsAllowsPropagating : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
// CHECK: bb0(
// CHECK: hop_to_executor
// CHECK: hop_to_executor
// CHECK: hop_to_executor
// CHECK-NOT: hop_to_executor
// CHECK: } // end sil function 'callerIsolationInheritingStopsAllowsPropagating'
sil [ossa] @callerIsolationInheritingStopsAllowsPropagating : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
Expand All @@ -416,14 +415,13 @@ bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyAc
// hop_to_executor due to ehre elase. We do eliminate one of the hop_to_executor
// though.
//
// CHECK: sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
// CHECK: sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@sil_isolated @guaranteed MyActor) -> () {
// CHECK: bb0(
// CHECK-NEXT: hop_to_executor
// CHECK-NEXT: tuple
// CHECK-NEXT: return
// CHECK: } // end sil function 'callerIsolationInheritingStopsReturnDeadEnd'
sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@guaranteed MyActor, @guaranteed MyActor, @guaranteed MyActor) -> () {
bb0(%0 : @guaranteed $MyActor, %1 : @guaranteed $MyActor, %2 : @guaranteed $MyActor):
sil [isolation "caller_isolation_inheriting"] [ossa] @callerIsolationInheritingStopsReturnDeadEnd : $@convention(thin) @async (@sil_isolated @guaranteed MyActor) -> () {
bb0(%0 : @guaranteed $MyActor):
hop_to_executor %0
hop_to_executor %0
%9999 = tuple ()
Expand Down