[concurrency] NFC refactor out some code from LowerHopToActor before changing the pass.

Specifically, I am refactoring out the code that converts actor/Optional<any
Actor> to an executor in preparation for adding code to LowerHopToExecutor that
handles Builtin.ImplicitIsolationActor.

The only actual functional change is that I made getExecutorForOptionalActor
support being invoked when generating code (i.e. when its SILBuilder has an
insertion point at the end of the block). It previously assumed that it would
always have a real SILInstruction as an insertion point. The changes can be seen
in the places where we now check if the insertion point equals the end of a
block. Its very minor and due to conditional control flow doesn't have any
actual impact given the manner that the code today is generated. This came up in
a subsequent commit when I reuse this code to generate a helper function for
converting Builtin.ImplicitIsolationActor to Builtin.Executor.

(cherry picked from commit 21915ae)

Conflicts:
	lib/SILOptimizer/Mandatory/LowerHopToActor.cpp

Author: gottesmm

lib/SILOptimizer/Mandatory/LowerHopToActor.cpp

@@ -51,7 +51,6 @@ namespace {
 /// inserts the derivations, turning those hops into hops to executors.
 /// IRGen expects hops to be to executors before it runs.
 class LowerHopToActor {
-  SILFunction *F;
   DominanceInfo *Dominance;
 
   /// A map from an actor value to the dominating instruction that
@@ -72,11 +71,7 @@ class LowerHopToActor {
                            SILValue actor, bool makeOptional);
 
 public:
-  LowerHopToActor(SILFunction *f,
-                  DominanceInfo *dominance)
-    : F(f),
-      Dominance(dominance)
-      { }
+  LowerHopToActor(DominanceInfo *dominance) : Dominance(dominance) {}
 
   /// The entry point to the transformation.
   bool run();
@@ -194,6 +189,118 @@ static AccessorDecl *getUnownedExecutorGetter(ASTContext &ctx,
   return nullptr;
 }
 
+/// Emit the instructions to derive an executor value from an actor value.
+static SILValue getExecutorForActor(SILBuilder &B, SILLocation loc,
+                                    SILValue actor) {
+  auto *F = actor->getFunction();
+  auto &ctx = F->getASTContext();
+  auto executorType = SILType::getPrimitiveObjectType(ctx.TheExecutorType);
+
+  // If the actor type is a default actor, go ahead and devirtualize here.
+  auto module = F->getModule().getSwiftModule();
+  CanType actorType = actor->getType().getASTType();
+
+  // Determine if the actor is a "default actor" in which case we'll build a
+  // default actor executor ref inline, rather than calling out to the
+  // user-provided executor function.
+  if (isDefaultActorType(actorType, module, F->getResilienceExpansion())) {
+    auto builtinName = ctx.getIdentifier(
+        getBuiltinName(BuiltinValueKind::BuildDefaultActorExecutorRef));
+    auto builtinDecl = cast<FuncDecl>(getBuiltinValueDecl(ctx, builtinName));
+    auto subs = SubstitutionMap::get(builtinDecl->getGenericSignature(),
+                                     {actorType}, LookUpConformanceInModule());
+    return B.createBuiltin(loc, builtinName, executorType, subs, {actor});
+  }
+
+  // Otherwise, go through (Distributed)Actor.unownedExecutor.
+  auto actorKind = actorType->isDistributedActor()
+                       ? KnownProtocolKind::DistributedActor
+                       : KnownProtocolKind::Actor;
+  auto actorProtocol = ctx.getProtocol(actorKind);
+  auto req = getUnownedExecutorGetter(ctx, actorProtocol);
+  assert(req && "Concurrency library broken");
+  SILDeclRef fn(req, SILDeclRef::Kind::Func);
+
+  // Open an existential actor type.
+  if (actorType->isExistentialType()) {
+    actorType = ExistentialArchetypeType::get(actorType)->getCanonicalType();
+    SILType loweredActorType = F->getLoweredType(actorType);
+    actor = B.createOpenExistentialRef(loc, actor, loweredActorType);
+  }
+
+  auto actorConf = lookupConformance(actorType, actorProtocol);
+  assert(actorConf && "hop_to_executor with actor that doesn't conform to "
+                      "Actor or DistributedActor");
+
+  auto subs = SubstitutionMap::get(req->getGenericSignature(), {actorType},
+                                   {actorConf});
+  auto fnType = F->getModule().Types.getConstantFunctionType(*F, fn);
+
+  auto witness = B.createWitnessMethod(loc, actorType, actorConf, fn,
+                                       SILType::getPrimitiveObjectType(fnType));
+  auto witnessCall = B.createApply(loc, witness, subs, {actor});
+
+  // The protocol requirement returns an UnownedSerialExecutor; extract
+  // the Builtin.Executor from it.
+  auto executorDecl = ctx.getUnownedSerialExecutorDecl();
+  auto executorProps = executorDecl->getStoredProperties();
+  assert(executorProps.size() == 1);
+  return B.createStructExtract(loc, witnessCall, executorProps[0]);
+}
+
+static SILValue getExecutorForOptionalActor(SILBuilder &B, SILLocation loc,
+                                            SILValue actor) {
+  auto &ctx = B.getASTContext();
+  auto executorType = SILType::getPrimitiveObjectType(ctx.TheExecutorType);
+  auto optionalExecutorType = SILType::getOptionalType(executorType);
+
+  // Unwrap the optional and call 'unownedExecutor'.
+  auto *someDecl = ctx.getOptionalSomeDecl();
+  auto *curBB = B.getInsertionBB();
+  auto *contBB = B.getInsertionPoint() == curBB->end()
+                     ? B.getFunction().createBasicBlockAfter(curBB)
+                     : curBB->split(B.getInsertionPoint());
+  auto *someBB = B.getFunction().createBasicBlockAfter(curBB);
+  auto *noneBB = B.getFunction().createBasicBlockAfter(someBB);
+
+  // unmarked executor
+  SILValue result = contBB->createPhiArgument(optionalExecutorType,
+                                              actor->getOwnershipKind());
+
+  SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 1> caseBBs;
+  caseBBs.push_back(std::make_pair(someDecl, someBB));
+  B.setInsertionPoint(curBB);
+  auto *switchEnum = B.createSwitchEnum(loc, actor, noneBB, caseBBs);
+
+  SILValue unwrappedActor;
+  if (B.hasOwnership()) {
+    unwrappedActor = switchEnum->createOptionalSomeResult();
+    B.setInsertionPoint(someBB);
+  } else {
+    B.setInsertionPoint(someBB);
+    unwrappedActor = B.createUncheckedEnumData(loc, actor, someDecl);
+  }
+
+  // Call 'unownedExecutor' in the some block and wrap the result into
+  // an optional.
+  SILValue unwrappedExecutor = getExecutorForActor(B, loc, unwrappedActor);
+  SILValue someValue =
+      B.createOptionalSome(loc, unwrappedExecutor, optionalExecutorType);
+  B.createBranch(loc, contBB, {someValue});
+
+  // In the none case, create a nil executor value, which represents
+  // the generic executor.
+  B.setInsertionPoint(noneBB);
+  SILValue noneValue = B.createOptionalNone(loc, optionalExecutorType);
+  B.createBranch(loc, contBB, {noneValue});
+  if (contBB->begin() == contBB->end()) {
+    B.setInsertionPoint(contBB);
+  } else {
+    B.setInsertionPoint(contBB->begin());
+  }
+  return result;
+}
+
 SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
                                           SILLocation loc, SILValue actor,
                                           bool makeOptional) {
@@ -204,125 +311,31 @@ SILValue LowerHopToActor::emitGetExecutor(SILBuilderWithScope &B,
   // If the operand is already a BuiltinExecutorType, just wrap it
   // in an optional.
   if (makeOptional && actor->getType().is<BuiltinExecutorType>()) {
-    return B.createOptionalSome(
-        loc, actor,
-        SILType::getOptionalType(actor->getType()));
+    return B.createOptionalSome(loc, actor,
+                                SILType::getOptionalType(actor->getType()));
   }
 
-  auto &ctx = F->getASTContext();
-  auto executorType = SILType::getPrimitiveObjectType(ctx.TheExecutorType);
-  auto optionalExecutorType = SILType::getOptionalType(executorType);
-
-  /// Emit the instructions to derive an executor value from an actor value.
-  auto getExecutorFor = [&](SILValue actor) -> SILValue {
-    // If the actor type is a default actor, go ahead and devirtualize here.
-    auto module = F->getModule().getSwiftModule();
-    CanType actorType = actor->getType().getASTType();
-
-    // Determine if the actor is a "default actor" in which case we'll build a default
-    // actor executor ref inline, rather than calling out to the user-provided executor function.
-    if (isDefaultActorType(actorType, module, F->getResilienceExpansion())) {
-      auto builtinName = ctx.getIdentifier(
-        getBuiltinName(BuiltinValueKind::BuildDefaultActorExecutorRef));
-      auto builtinDecl = cast<FuncDecl>(getBuiltinValueDecl(ctx, builtinName));
-      auto subs = SubstitutionMap::get(builtinDecl->getGenericSignature(),
-                                       {actorType},
-                                       LookUpConformanceInModule());
-      return B.createBuiltin(loc, builtinName, executorType, subs, {actor});
-    }
-
-    // Otherwise, go through (Distributed)Actor.unownedExecutor.
-    auto actorKind = actorType->isDistributedActor() ?
-                     KnownProtocolKind::DistributedActor :
-                     KnownProtocolKind::Actor;
-    auto actorProtocol = ctx.getProtocol(actorKind);
-    auto req = getUnownedExecutorGetter(ctx, actorProtocol);
-    assert(req && "Concurrency library broken");
-    SILDeclRef fn(req, SILDeclRef::Kind::Func);
-
-    // Open an existential actor type.
-    if (actorType->isExistentialType()) {
-      actorType = ExistentialArchetypeType::get(actorType)->getCanonicalType();
-      SILType loweredActorType = F->getLoweredType(actorType);
-      actor = B.createOpenExistentialRef(loc, actor, loweredActorType);
-    }
-
-    auto actorConf = lookupConformance(actorType, actorProtocol);
-    assert(actorConf &&
-           "hop_to_executor with actor that doesn't conform to Actor or DistributedActor");
-
-    auto subs = SubstitutionMap::get(req->getGenericSignature(),
-                                     {actorType}, {actorConf});
-    auto fnType = F->getModule().Types.getConstantFunctionType(*F, fn);
-
-    auto witness =
-      B.createWitnessMethod(loc, actorType, actorConf, fn,
-                            SILType::getPrimitiveObjectType(fnType));
-    auto witnessCall = B.createApply(loc, witness, subs, {actor});
-
-    // The protocol requirement returns an UnownedSerialExecutor; extract
-    // the Builtin.Executor from it.
-    auto executorDecl = ctx.getUnownedSerialExecutorDecl();
-    auto executorProps = executorDecl->getStoredProperties();
-    assert(executorProps.size() == 1);
-    return B.createStructExtract(loc, witnessCall, executorProps[0]);
-  };
-
   bool needEndBorrow = false;
   SILValue unmarkedExecutor;
   if (auto wrappedActor = actorType->getOptionalObjectType()) {
     assert(makeOptional);
-
-    if (B.hasOwnership() && actor->getOwnershipKind() == OwnershipKind::Owned) {
+    if (B.hasOwnership() &&
+        actor->getOwnershipKind() != OwnershipKind::Guaranteed) {
       actor = B.createBeginBorrow(loc, actor);
       needEndBorrow = true;
     }
 
-    // Unwrap the optional and call 'unownedExecutor'.
-    auto *someDecl = B.getASTContext().getOptionalSomeDecl();
-    auto *curBB = B.getInsertionPoint()->getParent();
-    auto *contBB = curBB->split(B.getInsertionPoint());
-    auto *someBB = B.getFunction().createBasicBlockAfter(curBB);
-    auto *noneBB = B.getFunction().createBasicBlockAfter(someBB);
-
-    unmarkedExecutor = contBB->createPhiArgument(
-        optionalExecutorType, actor->getOwnershipKind());
-
-    SmallVector<std::pair<EnumElementDecl *, SILBasicBlock *>, 1> caseBBs;
-    caseBBs.push_back(std::make_pair(someDecl, someBB));
-    B.setInsertionPoint(curBB);
-    auto *switchEnum = B.createSwitchEnum(loc, actor, noneBB, caseBBs);
-
-    SILValue unwrappedActor;
-    if (B.hasOwnership()) {
-      unwrappedActor = switchEnum->createOptionalSomeResult();
-      B.setInsertionPoint(someBB);
-    } else {
-      B.setInsertionPoint(someBB);
-      unwrappedActor = B.createUncheckedEnumData(loc, actor, someDecl);
-    }
-
-    // Call 'unownedExecutor' in the some block and wrap the result into
-    // an optional.
-    SILValue unwrappedExecutor = getExecutorFor(unwrappedActor);
-    SILValue someValue =
-        B.createOptionalSome(loc, unwrappedExecutor, optionalExecutorType);
-    B.createBranch(loc, contBB, {someValue});
-
-    // In the none case, create a nil executor value, which represents
-    // the generic executor.
-    B.setInsertionPoint(noneBB);
-    SILValue noneValue = B.createOptionalNone(loc, optionalExecutorType);
-    B.createBranch(loc, contBB, {noneValue});
-    B.setInsertionPoint(contBB->begin());
+    unmarkedExecutor = getExecutorForOptionalActor(B, loc, actor);
   } else {
-    unmarkedExecutor = getExecutorFor(actor);
+    unmarkedExecutor = getExecutorForActor(B, loc, actor);
+  }
 
-    // Inject the result into an optional if requested.
-    if (makeOptional) {
-      unmarkedExecutor = B.createOptionalSome(loc, unmarkedExecutor,
-          SILType::getOptionalType(unmarkedExecutor->getType()));
-    }
+  // Inject the result into an optional if requested and if our executor is not
+  // yet optional.
+  if (makeOptional && !unmarkedExecutor->getType().getOptionalObjectType()) {
+    unmarkedExecutor = B.createOptionalSome(
+        loc, unmarkedExecutor,
+        SILType::getOptionalType(unmarkedExecutor->getType()));
   }
 
   // Mark the dependence of the resulting value on the actor value to
@@ -342,7 +355,7 @@ class LowerHopToActorPass : public SILFunctionTransform {
   void run() override {
     auto fn = getFunction();
     auto domTree = getAnalysis<DominanceAnalysis>()->get(fn);
-    LowerHopToActor pass(getFunction(), domTree);
+    LowerHopToActor pass(domTree);
     if (pass.run())
       invalidateAnalysis(SILAnalysis::InvalidationKind::BranchesAndInstructions);
   }