Rewrite hop_to_executor so that it takes a Builtin.Executor in IRGen

rjmccall · rjmccall · commit 4f6f8b337706 · 2021-03-30T20:08:41.000-04:00
The comment in LowerHopToActor explains the design here.
We want SILGen to emit hops to actors, ignoring executors,
because it's easier to fully optimize in a world where deriving
an executor is a non-trivial operation.  But we also want something
prior to IRGen to lower the executor derivation because there are
useful static optimizations we can do, such as doing the derivation
exactly once on a dominance path and strength-reducing the derivation
(e.g. exploiting static knowledge that an actor is a default actor).

There are probably phase-ordering problems with doing this so late,
but hopefully they're restricted to situations like actors that
share an executor.  We'll want to optimize that eventually, but
in the meantime, this unblocks the executor work.
diff --git a/docs/SIL.rst b/docs/SIL.rst
@@ -3302,7 +3302,7 @@ hop_to_executor
 
   hop_to_executor %0 : $T
 
-  // $T must conform to the Actor protocol
+  // $T must be Builtin.Executor or conform to the Actor protocol
 
 Ensures that all instructions, which need to run on the actor's executor
 actually run on that executor.
@@ -3312,6 +3312,11 @@ Checks if the current executor is the one which is bound to the operand actor.
 If not, begins a suspension point and enqueues the continuation to the executor
 which is bound to the operand actor.
 
+SIL generation emits this instruction with operands of actor type as
+well as of type ``Builtin.Executor``.  The former are expected to be
+lowered by the SIL pipeline, so that IR generation only operands of type
+``Builtin.Executor`` remain.
+
 The operand is a guaranteed operand, i.e. not consumed.
 
 dealloc_stack
diff --git a/include/swift/AST/Builtins.def b/include/swift/AST/Builtins.def
@@ -735,6 +735,10 @@ BUILTIN_MISC_OPERATION(ResumeThrowingContinuationReturning,
 BUILTIN_MISC_OPERATION(ResumeThrowingContinuationThrowing,
                        "resumeThrowingContinuationThrowing", "", Special)
 
+/// Build a Builtin.Executor value from the given serial executor reference.
+BUILTIN_MISC_OPERATION(BuildSerialExecutorRef,
+                       "buildSerialExecutorRef", "", Special)
+
 // BUILTIN_MISC_OPERATION_WITH_SILGEN - Miscellaneous operations that are
 // specially emitted during SIL generation.
 //
diff --git a/include/swift/SIL/SILCloner.h b/include/swift/SIL/SILCloner.h
@@ -3053,7 +3053,7 @@ ::visitHopToExecutorInst(HopToExecutorInst *Inst) {
   recordClonedInstruction(Inst,
                           getBuilder().createHopToExecutor(
                             getOpLocation(Inst->getLoc()),
-                            getOpValue(Inst->getActor())));
+                            getOpValue(Inst->getTargetExecutor())));
 }
 
 
diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h
@@ -3474,11 +3474,12 @@ class HopToExecutorInst
 {
   friend SILBuilder;
 
-  HopToExecutorInst(SILDebugLocation DebugLoc, SILValue Actor, bool HasOwnership)
-      : UnaryInstructionBase(DebugLoc, Actor) { }
+  HopToExecutorInst(SILDebugLocation debugLoc, SILValue executor,
+                    bool hasOwnership)
+      : UnaryInstructionBase(debugLoc, executor) { }
 
 public:
-  SILValue getActor() const { return getOperand(); }
+  SILValue getTargetExecutor() const { return getOperand(); }
 };
 
 /// Instantiates a key path object.
diff --git a/include/swift/SILOptimizer/PassManager/Passes.def b/include/swift/SILOptimizer/PassManager/Passes.def
@@ -210,6 +210,8 @@ PASS(HighLevelLICM, "high-level-licm",
      "Loop Invariant Code Motion in High-Level SIL")
 PASS(IVInfoPrinter, "iv-info-printer",
      "Print Induction Variable Information for Testing")
+PASS(LowerHopToActor, "lower-hop-to-actor",
+     "Lower hop_to_executor instructions with actor operands")
 PASS(OptimizeHopToExecutor, "optimize-hop-to-executor",
      "Optimize hop_to_executor instructions for actor isolated code")
 PASS(InstCount, "inst-count",
diff --git a/lib/AST/Builtins.cpp b/lib/AST/Builtins.cpp
@@ -1481,6 +1481,15 @@ static ValueDecl *getResumeContinuationThrowing(ASTContext &ctx,
                             _void);
 }
 
+static ValueDecl *getBuildSerialExecutorRef(ASTContext &ctx, Identifier id) {
+  // TODO: restrict the generic parameter to the SerialExecutor protocol
+  return getBuiltinFunction(ctx, id, _thin,
+                            _generics(_unrestricted,
+                                      _layout(_typeparam(0), _classLayout())),
+                            _parameters(_typeparam(0)),
+                            _executor);
+}
+
 static ValueDecl *getAutoDiffCreateLinearMapContext(ASTContext &ctx,
                                                     Identifier id) {
   return getBuiltinFunction(
@@ -2658,6 +2667,9 @@ ValueDecl *swift::getBuiltinValueDecl(ASTContext &Context, Identifier Id) {
   case BuiltinValueKind::ConvertTaskToJob:
     return getConvertTaskToJob(Context, Id);
 
+  case BuiltinValueKind::BuildSerialExecutorRef:
+    return getBuildSerialExecutorRef(Context, Id);
+
   case BuiltinValueKind::PoundAssert:
     return getPoundAssert(Context, Id);
 
diff --git a/lib/IRGen/GenBuiltin.cpp b/lib/IRGen/GenBuiltin.cpp
@@ -306,6 +306,14 @@ void irgen::emitBuiltinCall(IRGenFunction &IGF, const BuiltinInfo &Builtin,
     return;
   }
 
+  if (Builtin.ID == BuiltinValueKind::BuildSerialExecutorRef) {
+    auto executor = args.claimNext();
+    executor = IGF.Builder.CreateBitCast(executor,
+                                         IGF.IGM.SwiftExecutorPtrTy);
+    out.add(executor);
+    return;
+  }
+
   // If this is an LLVM IR intrinsic, lower it to an intrinsic call.
   const IntrinsicInfo &IInfo = IGF.getSILModule().getIntrinsicInfo(FnId);
   llvm::Intrinsic::ID IID = IInfo.ID;
diff --git a/lib/IRGen/IRGenSIL.cpp b/lib/IRGen/IRGenSIL.cpp
@@ -6134,6 +6134,7 @@ void IRGenSILFunction::visitCheckedCastAddrBranchInst(
 }
 
 void IRGenSILFunction::visitHopToExecutorInst(HopToExecutorInst *i) {
+  assert(i->getTargetExecutor()->getType().is<BuiltinExecutorType>());
   llvm::Value *resumeFn = Builder.CreateIntrinsicCall(
           llvm::Intrinsic::coro_async_resume, {});
           
diff --git a/lib/SIL/IR/OperandOwnership.cpp b/lib/SIL/IR/OperandOwnership.cpp
@@ -814,6 +814,8 @@ BUILTIN_OPERAND_OWNERSHIP(ForwardingBorrow, AutoDiffProjectTopLevelSubcontext)
 // ownership should be 'TrivialUse'.
 BUILTIN_OPERAND_OWNERSHIP(ForwardingConsume, ConvertTaskToJob)
 
+BUILTIN_OPERAND_OWNERSHIP(BitwiseEscape, BuildSerialExecutorRef)
+
 BUILTIN_OPERAND_OWNERSHIP(TrivialUse, AutoDiffCreateLinearMapContext)
 
 #undef BUILTIN_OPERAND_OWNERSHIP
diff --git a/lib/SIL/IR/SILPrinter.cpp b/lib/SIL/IR/SILPrinter.cpp
@@ -2144,7 +2144,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {
   }
 
   void visitHopToExecutorInst(HopToExecutorInst *HTEI) {
-    *this << getIDAndType(HTEI->getActor());
+    *this << getIDAndType(HTEI->getTargetExecutor());
   }
 
   void visitSwitchValueInst(SwitchValueInst *SII) {
diff --git a/lib/SIL/IR/ValueOwnership.cpp b/lib/SIL/IR/ValueOwnership.cpp
@@ -552,6 +552,7 @@ CONSTANT_OWNERSHIP_BUILTIN(None, GetCurrentExecutor)
 CONSTANT_OWNERSHIP_BUILTIN(None, ResumeNonThrowingContinuationReturning)
 CONSTANT_OWNERSHIP_BUILTIN(None, ResumeThrowingContinuationReturning)
 CONSTANT_OWNERSHIP_BUILTIN(None, ResumeThrowingContinuationThrowing)
+CONSTANT_OWNERSHIP_BUILTIN(None, BuildSerialExecutorRef)
 
 #undef CONSTANT_OWNERSHIP_BUILTIN
 
diff --git a/lib/SIL/Verifier/SILVerifier.cpp b/lib/SIL/Verifier/SILVerifier.cpp
@@ -1851,6 +1851,16 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
               "default-actor builtin can only operate on default actors");
       return;
     }
+
+    if (builtinKind == BuiltinValueKind::BuildSerialExecutorRef) {
+      requireObjectType(BuiltinExecutorType, BI,
+                        "result of buildSerialExecutorRef");
+      auto arguments = BI->getArguments();
+      require(arguments.size() == 1,
+              "buildSerialExecutorRef expects one argument");
+      require(arguments[0]->getType().isObject(),
+              "operand of buildSerialExecutorRef should have object type");
+    }
   }
   
   void checkFunctionRefBaseInst(FunctionRefBaseInst *FRI) {
@@ -4772,6 +4782,15 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
           "result must match all parameters of invoke function but the first");
     }
   }
+
+  void checkHopToExecutorInst(HopToExecutorInst *HI) {
+    auto executor = HI->getTargetExecutor();
+    if (HI->getModule().getStage() == SILStage::Lowered) {
+      requireObjectType(BuiltinExecutorType, executor->getType(),
+                        "hop_to_executor instruction must take a "
+                        "Builtin.Executor in lowered SIL");
+    }
+  }
   
   void checkObjCProtocolInst(ObjCProtocolInst *OPI) {
     require(OPI->getProtocol()->isObjC(),
diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp
@@ -4406,12 +4406,12 @@ RValue SILGenFunction::emitApply(ResultPlanPtr &&resultPlan,
       breadcrumb = emitHopToTargetActor(loc, getActorIsolation(funcDecl),
                                         actorSelf);
     }
-  } else if (actor && substFnType->isAsync()) {
+  } else if (ExpectedExecutor && substFnType->isAsync()) {
     // Otherwise, if we're in an actor method ourselves, and we're calling into
     // any sort of async function, we'll want to make sure to hop back to our
     // own executor afterward, since the callee could have made arbitrary hops
     // out of our isolation domain.
-    breadcrumb = ExecutorBreadcrumb(actor);
+    breadcrumb = ExecutorBreadcrumb(ExpectedExecutor);
   }
 
   SILValue rawDirectResult;
diff --git a/lib/SILGen/SILGenFunction.h b/lib/SILGen/SILGenFunction.h
@@ -453,11 +453,11 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
   /// The metatype argument to an allocating constructor, if we're emitting one.
   SILValue AllocatorMetatype;
 
-  /// If set, the current function is an async function which is isolated to
-  /// this actor.
-  /// If set, hop_to_executor instructions must be inserted at the begin of the
-  /// function and after all suspension points.
-  SILValue actor;
+  /// If set, the current function is an async function which is formally
+  /// isolated to the given executor, and hop_to_executor instructions must
+  /// be inserted at the begin of the function and after all suspension
+  /// points.
+  SILValue ExpectedExecutor;
 
   /// True if 'return' without an operand or falling off the end of the current
   /// function is valid.
@@ -842,15 +842,17 @@ class LLVM_LIBRARY_VISIBILITY SILGenFunction
                             Optional<ManagedValue> actorSelf);
 
   /// Gets a reference to the current executor for the task.
+  /// \returns a value of type Builtin.Executor
   SILValue emitGetCurrentExecutor(SILLocation loc);
   
-  /// Generates code to obtain the executor given the actor's decl.
-  /// \returns a SILValue representing the executor.
-  SILValue emitLoadActorExecutor(VarDecl *actorDecl);
+  /// Generates code to obtain an actor's executor given a reference
+  /// to the actor.
+  /// \returns a value which can be used with hop_to_executor
+  SILValue emitLoadActorExecutor(SILLocation loc, ManagedValue actor);
 
   /// Generates the code to obtain the executor for the shared instance 
   /// of the \p globalActor based on the type.
-  /// \returns a SILValue representing the executor.
+  /// \returns a value which can be used with hop_to_executor
   SILValue emitLoadGlobalActorExecutor(Type globalActor);
 
   //===--------------------------------------------------------------------===//
diff --git a/lib/SILGen/SILGenProlog.cpp b/lib/SILGen/SILGenProlog.cpp
@@ -465,59 +465,59 @@ void SILGenFunction::emitProlog(CaptureInfo captureInfo,
   if (!F.isAsync())
     return;
 
-  // Initialize 'actor' if the function is an actor-isolated function or
-  // closure.
+  // Initialize ExpectedExecutor if the function is an actor-isolated
+  // function or closure.
   if (auto *funcDecl =
         dyn_cast_or_null<AbstractFunctionDecl>(FunctionDC->getAsDecl())) {
     auto actorIsolation = getActorIsolation(funcDecl);
     switch (actorIsolation.getKind()) {
-      case ActorIsolation::Unspecified:
-      case ActorIsolation::Independent:
-      case ActorIsolation::GlobalActorUnsafe:
-        break;
-
-      case ActorIsolation::ActorInstance: {
-        assert(selfParam && "no self parameter for ActorInstance isolation");
-        ManagedValue selfArg = ManagedValue::forUnmanaged(F.getSelfArgument());
-        actor = selfArg
-                    .borrow(*this, RegularLocation::getAutoGeneratedLocation(
-                                       F.getLocation()))
-                    .getValue();
-        break;
-      }
+    case ActorIsolation::Unspecified:
+    case ActorIsolation::Independent:
+    case ActorIsolation::GlobalActorUnsafe:
+      break;
+
+    case ActorIsolation::ActorInstance: {
+      assert(selfParam && "no self parameter for ActorInstance isolation");
+      auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
+      ManagedValue selfArg = ManagedValue::forUnmanaged(F.getSelfArgument());
+      ExpectedExecutor = emitLoadActorExecutor(loc, selfArg);
+      break;
+    }
 
-      case ActorIsolation::GlobalActor:
-        actor = emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
-        break;
+    case ActorIsolation::GlobalActor:
+      ExpectedExecutor =
+        emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
+      break;
     }
   } else if (auto *closureExpr = dyn_cast<AbstractClosureExpr>(FunctionDC)) {
     auto actorIsolation = closureExpr->getActorIsolation();
     switch (actorIsolation.getKind()) {
-      case ClosureActorIsolation::Independent:
-        break;
-
-      case ClosureActorIsolation::ActorInstance:
-        actor = emitLoadActorExecutor(actorIsolation.getActorInstance());
-        break;
+    case ClosureActorIsolation::Independent:
+      break;
+
+    case ClosureActorIsolation::ActorInstance: {
+      auto loc = RegularLocation::getAutoGeneratedLocation(F.getLocation());
+      auto actorDecl = actorIsolation.getActorInstance();
+      Type actorType = actorDecl->getType();
+      RValue actorInstanceRV = emitRValueForDecl(loc,
+        actorDecl, actorType, AccessSemantics::Ordinary);
+      ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
+      ExpectedExecutor = emitLoadActorExecutor(loc, actorInstance);
+      break;
+    }
 
-      case ClosureActorIsolation::GlobalActor:
-        actor = emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
-        break;
+    case ClosureActorIsolation::GlobalActor:
+      ExpectedExecutor =
+        emitLoadGlobalActorExecutor(actorIsolation.getGlobalActor());
+      break;
     }
   }
   
-  // Jump to our actor's executor if we're isolated to one.
-  if (actor)
+  // Jump to the expected executor.
+  if (ExpectedExecutor)
     B.createHopToExecutor(
                     RegularLocation::getAutoGeneratedLocation(F.getLocation()),
-                    actor);
-}
-
-SILValue SILGenFunction::emitLoadActorExecutor(VarDecl *actorDecl) {
-  RValue actorInstanceRV = emitRValueForDecl(F.getLocation(),
-    actorDecl, actorDecl->getType(), AccessSemantics::Ordinary);
-  ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
-  return actorInstance.borrow(*this, F.getLocation()).getValue();
+                    ExpectedExecutor);
 }
 
 SILValue SILGenFunction::emitLoadGlobalActorExecutor(Type globalActor) {
@@ -546,11 +546,20 @@ SILValue SILGenFunction::emitLoadGlobalActorExecutor(Type globalActor) {
     actorType, /*isSuper*/ false, sharedInstanceDecl, PreparedArguments(),
     subs, AccessSemantics::Ordinary, instanceType, SGFContext());
   ManagedValue actorInstance = std::move(actorInstanceRV).getScalarValue();
+  return emitLoadActorExecutor(loc, actorInstance);
+}
 
+SILValue SILGenFunction::emitLoadActorExecutor(SILLocation loc,
+                                               ManagedValue actor) {
+  SILValue actorV;
   if (isInFormalEvaluationScope())
-    return actorInstance.formalAccessBorrow(*this, loc).getValue();
+    actorV = actor.formalAccessBorrow(*this, loc).getValue();
   else
-    return actorInstance.borrow(*this, loc).getValue();
+    actorV = actor.borrow(*this, loc).getValue();
+
+  // For now, we just want to emit a hop_to_executor directly to the
+  // actor; LowerHopToActor will add the emission logic necessary later.
+  return actorV;
 }
 
 ExecutorBreadcrumb SILGenFunction::emitHopToTargetActor(SILLocation loc,
@@ -571,10 +580,7 @@ ExecutorBreadcrumb SILGenFunction::emitHopToTargetActor(SILLocation loc,
     // "self" here means the actor instance's "self" value.
     assert(maybeSelf.hasValue() && "actor-instance but no self provided?");
     auto self = maybeSelf.getValue();
-    if (isInFormalEvaluationScope())
-      executor = self.formalAccessBorrow(*this, loc).getValue();
-    else
-      executor = self.borrow(*this, loc).getValue();
+    executor = emitLoadActorExecutor(loc, self);
     break;
   }
 
@@ -607,8 +613,8 @@ void ExecutorBreadcrumb::emit(SILGenFunction &SGF, SILLocation loc) {
 SILValue SILGenFunction::emitGetCurrentExecutor(SILLocation loc) {
   // If this is an actor method, then the actor is the only executor we should
   // be running on (if we aren't setting up for a cross-actor call).
-  if (actor)
-    return actor;
+  if (ExpectedExecutor)
+    return ExpectedExecutor;
   
   // Otherwise, we'll have to ask the current task what executor it's running
   // on.
diff --git a/lib/SILOptimizer/Mandatory/CMakeLists.txt b/lib/SILOptimizer/Mandatory/CMakeLists.txt
@@ -15,6 +15,7 @@ target_sources(swiftSILOptimizer PRIVATE
   DiagnoseUnreachable.cpp
   Differentiation.cpp
   IRGenPrepare.cpp
+  LowerHopToActor.cpp
   MandatoryInlining.cpp
   NestedSemanticFunctionCheck.cpp
   OptimizeHopToExecutor.cpp
diff --git a/lib/SILOptimizer/Mandatory/LowerHopToActor.cpp b/lib/SILOptimizer/Mandatory/LowerHopToActor.cpp
diff --git a/lib/SILOptimizer/PassManager/PassPipeline.cpp b/lib/SILOptimizer/PassManager/PassPipeline.cpp
diff --git a/lib/SILOptimizer/Transforms/AccessEnforcementReleaseSinking.cpp b/lib/SILOptimizer/Transforms/AccessEnforcementReleaseSinking.cpp
diff --git a/test/IRGen/async/hop_to_executor.sil b/test/IRGen/async/hop_to_executor.sil
diff --git a/test/SILOptimizer/lower_hop_to_actor.sil b/test/SILOptimizer/lower_hop_to_actor.sil

Original file line number	Diff line number	Diff line change
`@@ -3053,7 +3053,7 @@ ::visitHopToExecutorInst(HopToExecutorInst *Inst) {`
`3053`	`3053`	`recordClonedInstruction(Inst,`
`3054`	`3054`	`getBuilder().createHopToExecutor(`
`3055`	`3055`	`getOpLocation(Inst->getLoc()),`
`3056`		`- getOpValue(Inst->getActor())));`
	`3056`	`+ getOpValue(Inst->getTargetExecutor())));`
`3057`	`3057`	`}`
`3058`	`3058`
`3059`	`3059`
Original file line number	Diff line number	Diff line change
`@@ -6134,6 +6134,7 @@ void IRGenSILFunction::visitCheckedCastAddrBranchInst(`
`6134`	`6134`	`}`
`6135`	`6135`
`6136`	`6136`	`void IRGenSILFunction::visitHopToExecutorInst(HopToExecutorInst *i) {`
	`6137`	`+ assert(i->getTargetExecutor()->getType().is<BuiltinExecutorType>());`
`6137`	`6138`	`llvm::Value *resumeFn = Builder.CreateIntrinsicCall(`
`6138`	`6139`	`llvm::Intrinsic::coro_async_resume, {});`
`6139`	`6140`
Original file line number	Diff line number	Diff line change
`@@ -2144,7 +2144,7 @@ class SILPrinter : public SILInstructionVisitor<SILPrinter> {`
`2144`	`2144`	`}`
`2145`	`2145`
`2146`	`2146`	`void visitHopToExecutorInst(HopToExecutorInst *HTEI) {`
`2147`		`- *this << getIDAndType(HTEI->getActor());`
	`2147`	`+ *this << getIDAndType(HTEI->getTargetExecutor());`
`2148`	`2148`	`}`
`2149`	`2149`
`2150`	`2150`	`void visitSwitchValueInst(SwitchValueInst *SII) {`