[SILGen] Only hop to the callee isolation domain once when evaluating isolated

default arguments.

(cherry picked from commit 5ac839b)

Author: hborla

lib/SILGen/SILGenApply.cpp

@@ -2695,6 +2695,28 @@ class DelayedArgument {
     return LV().Loc;
   }
 
+  bool isDefaultArg() const {
+    return Kind == DefaultArgument;
+  }
+
+  SILLocation getDefaultArgLoc() const {
+    assert(isDefaultArg());
+    auto storage = Value.get<DefaultArgumentStorage>(Kind);
+    return storage.loc;
+  }
+
+  llvm::Optional<ActorIsolation> getIsolation() const {
+    if (!isDefaultArg())
+      return llvm::None;
+
+    auto storage = Value.get<DefaultArgumentStorage>(Kind);
+    if (!storage.implicitlyAsync)
+      return llvm::None;
+
+    auto callee = storage.defaultArgsOwner.getDecl();
+    return getActorIsolation(callee);
+  }
+
   void emit(SILGenFunction &SGF, SmallVectorImpl<ManagedValue> &args,
             size_t &argIndex) {
     switch (Kind) {
@@ -2920,6 +2942,31 @@ static void emitDelayedArguments(SILGenFunction &SGF,
                          MutableArrayRef<SmallVector<ManagedValue, 4>> args) {
   assert(!delayedArgs.empty());
 
+  // If any of the delayed arguments are isolated default arguments,
+  // argument evaluation happens in the following order:
+  //
+  //   1. Left-to-right evalution of explicit r-value arguments
+  //   2. Left-to-right evaluation of formal access arguments
+  //   3. Hop to the callee's isolation domain
+  //   4. Left-to-right evaluation of default arguments
+
+  // So, if any delayed arguments are isolated, all default arguments
+  // are collected during the first pass over the delayed arguments,
+  // and emitted separately after a hop to the callee's isolation domain.
+
+  llvm::Optional<ActorIsolation> defaultArgIsolation;
+  for (auto &arg : delayedArgs) {
+    if (auto isolation = arg.getIsolation()) {
+      defaultArgIsolation = isolation;
+      break;
+    }
+  }
+
+  SmallVector<std::tuple<
+      /*delayedArgIt*/decltype(delayedArgs)::iterator,
+      /*siteArgsIt*/decltype(args)::iterator,
+      /*index*/size_t>, 2> isolatedArgs;
+
   SmallVector<std::pair<SILValue, SILLocation>, 4> emittedInoutArgs;
   auto delayedNext = delayedArgs.begin();
 
@@ -2928,7 +2975,8 @@ static void emitDelayedArguments(SILGenFunction &SGF,
   // wherever there's a delayed argument to insert.
   //
   // Note that this also begins the formal accesses in evaluation order.
-  for (auto &siteArgs : args) {
+  for (auto argsIt = args.begin(); argsIt != args.end(); ++argsIt) {
+    auto &siteArgs = *argsIt;
     // NB: siteArgs.size() may change during iteration
     for (size_t i = 0; i < siteArgs.size(); ) {
       auto &siteArg = siteArgs[i];
@@ -2941,6 +2989,15 @@ static void emitDelayedArguments(SILGenFunction &SGF,
       assert(delayedNext != delayedArgs.end());
       auto &delayedArg = *delayedNext;
 
+      if (defaultArgIsolation && delayedArg.isDefaultArg()) {
+        isolatedArgs.push_back(std::make_tuple(delayedNext, argsIt, i));
+        if (++delayedNext == delayedArgs.end()) {
+          goto done;
+        } else {
+          continue;
+        }
+      }
+
       // Emit the delayed argument and replace it in the arguments array.
       delayedArg.emit(SGF, siteArgs, i);
 
@@ -2961,6 +3018,45 @@ static void emitDelayedArguments(SILGenFunction &SGF,
 
 done:
 
+  if (defaultArgIsolation) {
+    assert(SGF.F.isAsync());
+    assert(!isolatedArgs.empty());
+
+    auto &firstArg = *std::get<0>(isolatedArgs[0]);
+    auto loc = firstArg.getDefaultArgLoc();
+
+    SILValue executor;
+    switch (*defaultArgIsolation) {
+    case ActorIsolation::GlobalActor:
+    case ActorIsolation::GlobalActorUnsafe:
+      executor = SGF.emitLoadGlobalActorExecutor(
+          defaultArgIsolation->getGlobalActor());
+      break;
+
+    case ActorIsolation::ActorInstance:
+      llvm_unreachable("default arg cannot be actor instance isolated");
+
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Nonisolated:
+    case ActorIsolation::NonisolatedUnsafe:
+      llvm_unreachable("Not isolated");
+    }
+
+    // Hop to the target isolation domain once to evaluate all
+    // default arguments.
+    SGF.emitHopToTargetExecutor(loc, executor);
+
+    size_t argsEmitted = 0;
+    for (auto &isolatedArg : isolatedArgs) {
+      auto &delayedArg = *std::get<0>(isolatedArg);
+      auto &siteArgs = *std::get<1>(isolatedArg);
+      auto argIndex = std::get<2>(isolatedArg) + argsEmitted;
+      auto origIndex = argIndex;
+      delayedArg.emit(SGF, siteArgs, argIndex);
+      argsEmitted += (argIndex - origIndex);
+    }
+  }
+
   // Check to see if we have multiple inout arguments which obviously
   // alias.  Note that we could do this in a later SILDiagnostics pass
   // as well: this would be stronger (more equivalences exposed) but

lib/SILGen/SILGenExpr.cpp

@@ -2564,24 +2564,8 @@ SILGenFunction::emitApplyOfDefaultArgGenerator(SILLocation loc,
   emitCaptures(loc, generator, CaptureEmission::ImmediateApplication,
                captures);
 
-  // The default argument might require the callee's isolation. If so,
-  // make sure to emit an actor hop.
-  //
-  // FIXME: Instead of hopping back and forth for each individual isolated
-  // default argument, we should emit one hop for all default arguments if
-  // any of them are isolated, and immediately enter the function after.
-  llvm::Optional<ActorIsolation> implicitActorHopTarget = llvm::None;
-  if (implicitlyAsync) {
-    auto *param = getParameterAt(defaultArgsOwner.getDecl(), destIndex);
-    auto isolation = param->getInitializerIsolation();
-    if (isolation.isActorIsolated()) {
-      implicitActorHopTarget = isolation;
-    }
-  }
-
   return emitApply(std::move(resultPtr), std::move(argScope), loc, fnRef, subs,
-                   captures, calleeTypeInfo, ApplyOptions(), C,
-                   implicitActorHopTarget);
+                   captures, calleeTypeInfo, ApplyOptions(), C, llvm::None);
 }
 
 RValue SILGenFunction::emitApplyOfStoredPropertyInitializer(

test/Concurrency/isolated_default_argument_eval.swift

@@ -9,17 +9,62 @@ func requiresMainActor() -> Int { 0 }
 @MainActor
 func mainActorDefaultArg(value: Int = requiresMainActor()) {}
 
+@MainActor
+func mainActorMultiDefaultArg(x: Int = requiresMainActor(),
+                              y: Int = 0,
+                              tuple: (Int, Int) = (requiresMainActor(), 2),
+                              z: Int = 0) {}
+
 // CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval15mainActorCalleryyF
 @MainActor func mainActorCaller() {
   mainActorDefaultArg()
+  mainActorMultiDefaultArg()
 }
 
 // CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval22nonisolatedAsyncCalleryyYaF
 func nonisolatedAsyncCaller() async {
   // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
-  // CHECK: [[GETARG:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
   // CHECK: hop_to_executor {{.*}} : $MainActor
-  // CHECK-NEXT: apply [[GETARG]]()
-  // CHECK-NEXT: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
+  // CHECK: [[GET_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval19mainActorDefaultArg5valueySi_tFfA_
+  // CHECK-NEXT: apply [[GET_VALUE]]()
+  // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
   await mainActorDefaultArg()
+
+  // CHECK: hop_to_executor {{.*}} : $MainActor
+  // CHECK: [[GET_X:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA_
+  // CHECK-NEXT: apply [[GET_X]]()
+  // CHECK-NOT: hop_to_executor
+  // CHECK: [[GET_Y:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA0_
+  // CHECK-NEXT: apply [[GET_Y]]()
+  // CHECK-NOT: hop_to_executor
+  // CHECK: [[GET_TUPLE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA1_
+  // CHECK-NEXT: apply [[GET_TUPLE]]()
+  // CHECK-NOT: hop_to_executor
+  // CHECK: [[GET_Z:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval24mainActorMultiDefaultArg1x1y5tuple1zySi_S2i_SitSitFfA2_
+  // CHECK-NEXT: apply [[GET_Z]]()
+  // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
+  await mainActorMultiDefaultArg()
+}
+
+@MainActor
+func isolatedDefaultInoutMix(x: inout Int, y: Int, z: Int = requiresMainActor()) {}
+
+var argValue: Int { 0 }
+
+// CHECK-LABEL: sil hidden [ossa] @$s30isolated_default_argument_eval20passInoutWithDefaultyyYaF
+func passInoutWithDefault() async {
+  // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
+
+  var x = 0
+
+  // CHECK: [[GET_ARG_VALUE:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval8argValueSivg
+  // CHECK-NEXT: [[ARG_VALUE:%[0-9]+]] = apply [[GET_ARG_VALUE]]()
+  // CHECK-NEXT: [[INOUT_X:%[0-9]+]] = begin_access [modify]
+  // CHECK: hop_to_executor {{.*}} : $MainActor
+  // CHECK: [[GET_Z:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval0A15DefaultInoutMix1x1y1zySiz_S2itFfA1_
+  // CHECK-NEXT: [[Z:%[0-9]+]] = apply [[GET_Z]]()
+  // CHECK: [[FN:%[0-9]+]] = function_ref @$s30isolated_default_argument_eval0A15DefaultInoutMix1x1y1zySiz_S2itF : $@convention(thin) (@inout Int, Int, Int) -> ()
+  // CHECK: apply [[FN]]([[INOUT_X]], [[ARG_VALUE]], [[Z]])
+  // CHECK: hop_to_executor {{.*}} : $Optional<Builtin.Executor>
+  await isolatedDefaultInoutMix(x: &x, y: argValue)
 }