diff --git a/lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp b/lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp index 04527ce359640..d9680f40b2efe 100644 --- a/lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp +++ b/lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp @@ -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(&inst)) { @@ -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; } @@ -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(function->maybeGetIsolatedArgument()); + return actors.lookup(SILValue(fArg)); + } + + return BlockState::Unknown; + }(); state.intra = BlockState::NotSet; for (SILInstruction &inst : *state.block) { if (isSuspensionPoint(&inst)) { @@ -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(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; } @@ -403,6 +385,29 @@ bool OptimizeHopToExecutor::needsExecutor(SILInstruction *inst) { if (isa(inst) || isa(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(inst)) { + if (auto isolation = inst->getFunction()->getActorIsolation(); + isolation && isolation->isCallerIsolationInheriting()) { + return true; + } + } + return inst->mayReadOrWriteMemory(); } diff --git a/test/Concurrency/nonisolated_nonsending_optimize_hoptoexecutor.swift b/test/Concurrency/nonisolated_nonsending_optimize_hoptoexecutor.swift index 69cc4c4a22e0e..83e001e3e8774 100644 --- a/test/Concurrency/nonisolated_nonsending_optimize_hoptoexecutor.swift +++ b/test/Concurrency/nonisolated_nonsending_optimize_hoptoexecutor.swift @@ -3,14 +3,11 @@ // REQUIRES: concurrency // CHECK-LABEL: sil hidden [noinline] @$s4testAAyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional) -> () { -// 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) -> () { -// CHECK: hop_to_executor -// CHECK: hop_to_executor // CHECK: } // end sil function '$s4test5test2yyYaF' @inline(never) nonisolated(nonsending) func test2() async { @@ -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) -> () -// CHECK: hop_to_executor // CHECK: function_ref @$s4test5test2yyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional) -> () // CHECK: } // end sil function '$s4test6calleryyYaF' public func caller() async { diff --git a/test/SILOptimizer/optimize_hop_to_executor.sil b/test/SILOptimizer/optimize_hop_to_executor.sil index e1e7292536696..10620d4238b1f 100644 --- a/test/SILOptimizer/optimize_hop_to_executor.sil +++ b/test/SILOptimizer/optimize_hop_to_executor.sil @@ -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): @@ -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 ()