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

Specifically:

1. We assume in nonisolated(nonsending) that we are already on the relevant
actor. This lets us always eliminate the initial hop_to_executor.

2. We stopped treating nonisolated(nonsending) functions as suspension points
since we are guaranteed to always be on the same actor when we enter/return.

3. Now that nonisolated(nonsending) is no longer a suspension point, I could
sink the needs executor nonisolated(nonsending) specific code into the needs
executor code. For those unfamiliar it is that we: a. treat a
nonisolated(nonsending) callee as a needs executor since we are no longer
guaranteed to hop in callees and b. treat returns from nonisolated(nonsending)
functions as being a needs executor instruction since we are no longer
guaranteed to hop in the caller after such a function returns.

rdar://155465878

lib/SILOptimizer/Mandatory/OptimizeHopToExecutor.cpp

@@ -119,6 +119,12 @@ public:
 /// 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)) {
@@ -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<SILFunctionArgument>(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<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;
 }
@@ -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();
 }
 

test/Concurrency/isolated_nonsending_isolation_macro_sil.swift

@@ -12,14 +12,10 @@ func take(iso: (any Actor)?) {}
 // CHECK-LABEL: // nonisolatedNonsending()
 // CHECK-NEXT: // Isolation: caller_isolation_inheriting
 // CHECK-NEXT: sil hidden @$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF : $@convention(thin) @async (@sil_isolated @sil_implicit_leading_param @guaranteed Optional<any Actor>) -> () {
-// CHECK:      bb0(%0 : $Optional<any Actor>):
-// CHECK-NEXT:   hop_to_executor %0 // id: %1
-// CHECK-NEXT:   retain_value %0 // id: %2
-// CHECK-NEXT:   debug_value %0, let, name "iso" // id: %3
-// CHECK-NEXT:   // function_ref take(iso:)
-// CHECK-NEXT:   %4 = function_ref @$s39isolated_nonsending_isolation_macro_sil4take3isoyScA_pSg_tF : $@convention(thin) (@guaranteed Optional<any Actor>) -> () // user: %5
-// CHECK-NEXT:   %5 = apply %4(%0) : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
-// CHECK-NEXT:   release_value %0 // id: %6
-// CHECK-NEXT:   %7 = tuple () // user: %8
-// CHECK-NEXT:   return %7 // id: %8
-// CHECK-NEXT: } // end sil function '$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF'
+// CHECK:      bb0([[ACTOR:%.*]] : $Optional<any Actor>):
+// CHECK:   retain_value [[ACTOR]]
+// CHECK:   debug_value [[ACTOR]], let, name "iso"
+// CHECK:   [[FUNC:%.*]] = function_ref @$s39isolated_nonsending_isolation_macro_sil4take3isoyScA_pSg_tF : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
+// CHECK:   apply [[FUNC]]([[ACTOR]]) : $@convention(thin) (@guaranteed Optional<any Actor>) -> ()
+// CHECK:   release_value [[ACTOR]]
+// CHECK: } // end sil function '$s39isolated_nonsending_isolation_macro_sil21nonisolatedNonsendingyyYaF'

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<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 {
@@ -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 {

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 ()