[region-isolation] Split in the type system SILIsolationInfo that has been merged and those that haven't.

Specifically, I introduced a new composition type called
SILDynamicMergedIsolationInfo that just contains a
SILIsolationInfo. Importantly, whenever one merges a SILIsolationInfo with
another SILIsolationInfo, one gets back a SILDynamicMergedIsolationInfo.

The reason why I am doing this is that we drop nonisolated(unsafe) when merging
so I want to ensure that parts of the code that use merging (where the dropping
occurs) and normal SILIsolationInfo where we do not want to merge is
distinguished.

Author: gottesmm

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -160,7 +160,9 @@ class regionanalysisimpl::TrackableValueState {
   }
 
   void mergeIsolationRegionInfo(SILIsolationInfo newRegionInfo) {
-    regionInfo = getIsolationRegionInfo().merge(newRegionInfo);
+    // TODO: Remove this.
+    regionInfo =
+        getIsolationRegionInfo().merge(newRegionInfo).getIsolationInfo();
   }
 
   void setDisconnectedNonisolatedUnsafe() {

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -342,7 +342,7 @@ struct TransferringOperandState {
   /// The dynamic isolation info of the region of value when we transferred.
   ///
   /// This will contain the isolated value if we found one.
-  SILIsolationInfo isolationInfo;
+  SILDynamicMergedIsolationInfo isolationInfo;
 
   /// The dynamic isolation history at this point.
   IsolationHistory isolationHistory;
@@ -888,17 +888,16 @@ struct PartitionOpEvaluator {
   }
 
   /// Call handleTransferNonTransferrable on our CRTP subclass.
-  void
-  handleTransferNonTransferrable(const PartitionOp &op, Element elt,
-                                 SILIsolationInfo isolationRegionInfo) const {
+  void handleTransferNonTransferrable(
+      const PartitionOp &op, Element elt,
+      SILDynamicMergedIsolationInfo isolationRegionInfo) const {
     return asImpl().handleTransferNonTransferrable(op, elt,
                                                    isolationRegionInfo);
   }
   /// Just call our CRTP subclass.
-  void
-  handleTransferNonTransferrable(const PartitionOp &op, Element elt,
-                                 Element otherElement,
-                                 SILIsolationInfo isolationRegionInfo) const {
+  void handleTransferNonTransferrable(
+      const PartitionOp &op, Element elt, Element otherElement,
+      SILDynamicMergedIsolationInfo isolationRegionInfo) const {
     return asImpl().handleTransferNonTransferrable(op, elt, otherElement,
                                                    isolationRegionInfo);
   }
@@ -916,10 +915,10 @@ struct PartitionOpEvaluator {
   ///
   /// The bool result is if it is captured by a closure element. That only is
   /// computed if \p sourceOp is non-null.
-  std::pair<SILIsolationInfo, bool>
+  std::pair<SILDynamicMergedIsolationInfo, bool>
   getIsolationRegionInfo(Region region, Operand *sourceOp) const {
     bool isClosureCapturedElt = false;
-    SILIsolationInfo isolationRegionInfo;
+    SILDynamicMergedIsolationInfo isolationRegionInfo;
 
     for (const auto &pair : p.range()) {
       if (pair.second == region) {
@@ -1039,7 +1038,7 @@ struct PartitionOpEvaluator {
       // dynamic isolation region info found by the dataflow.
       Region transferredRegion = p.getRegion(transferredElement);
       bool isClosureCapturedElt = false;
-      SILIsolationInfo transferredRegionIsolation;
+      SILDynamicMergedIsolationInfo transferredRegionIsolation;
       std::tie(transferredRegionIsolation, isClosureCapturedElt) =
           getIsolationRegionInfo(transferredRegion, op.getSourceOp());
 
@@ -1189,7 +1188,7 @@ struct PartitionOpEvaluator {
   // use have the same isolation.
   void handleTransferNonTransferrableHelper(
       const PartitionOp &op, Element elt,
-      SILIsolationInfo dynamicMergedIsolationInfo) const {
+      SILDynamicMergedIsolationInfo dynamicMergedIsolationInfo) const {
     if (shouldTryToSquelchErrors()) {
       // If we have a temporary that is initialized with an unsafe nonisolated
       // value... squelch the error like if we were that value.
@@ -1252,13 +1251,13 @@ struct PartitionOpEvaluatorBaseImpl : PartitionOpEvaluator<Subclass> {
 
   /// This is called if we detect a never transferred element that was passed to
   /// a transfer instruction.
-  void handleTransferNonTransferrable(const PartitionOp &op, Element elt,
-                                      SILIsolationInfo regionInfo) const {}
+  void handleTransferNonTransferrable(
+      const PartitionOp &op, Element elt,
+      SILDynamicMergedIsolationInfo regionInfo) const {}
 
-  void
-  handleTransferNonTransferrable(const PartitionOp &op, Element elt,
-                                 Element otherElement,
-                                 SILIsolationInfo isolationRegionInfo) const {}
+  void handleTransferNonTransferrable(
+      const PartitionOp &op, Element elt, Element otherElement,
+      SILDynamicMergedIsolationInfo isolationRegionInfo) const {}
 
   /// This is used to determine if an element is actor derived. If we determine
   /// that a region containing such an element is transferred, we emit an error

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -102,6 +102,8 @@ class ActorInstance {
   }
 };
 
+class SILDynamicMergedIsolationInfo;
+
 class SILIsolationInfo {
 public:
   /// The lattice is:
@@ -230,7 +232,12 @@ class SILIsolationInfo {
     return (kind == Task || kind == Actor) && bool(isolatedValue);
   }
 
-  [[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;
+  /// Merge this SILIsolationInfo with \p other and produce a
+  /// SILDynamicMergedIsolationInfo that represents the dynamic isolation of a
+  /// value found by merging the value's region's isolation info. This causes
+  /// nonisolated(unsafe) to be dropped.
+  [[nodiscard]] SILDynamicMergedIsolationInfo
+  merge(SILIsolationInfo other) const;
 
   static SILIsolationInfo getDisconnected(bool isUnsafeNonIsolated) {
     return {Kind::Disconnected, isUnsafeNonIsolated};
@@ -374,6 +381,45 @@ class SILIsolationInfo {
   void Profile(llvm::FoldingSetNodeID &id) const;
 };
 
+/// A SILIsolationInfo that has gone through merging and represents the dynamic
+/// isolation info of a value found by merging its isolation at a region
+/// point. This means that nonisolated(unsafe) has been removed. It is used so
+/// that in the type system we can distinguish in between isolation info that is
+/// static isolation info associated with a value and dynamic isolation info
+/// that can just be used for dataflow.
+class SILDynamicMergedIsolationInfo {
+  SILIsolationInfo innerInfo;
+
+public:
+  SILDynamicMergedIsolationInfo() : innerInfo() {}
+  SILDynamicMergedIsolationInfo(SILIsolationInfo innerInfo)
+      : innerInfo(innerInfo) {}
+
+  [[nodiscard]] SILDynamicMergedIsolationInfo
+  merge(SILIsolationInfo other) const;
+
+  [[nodiscard]] SILDynamicMergedIsolationInfo
+  merge(SILDynamicMergedIsolationInfo other) const {
+    return merge(other.getIsolationInfo());
+  }
+
+  operator bool() const { return bool(innerInfo); }
+
+  SILIsolationInfo getIsolationInfo() const { return innerInfo; }
+
+  bool isDisconnected() const { return innerInfo.isDisconnected(); }
+
+  bool hasSameIsolation(SILIsolationInfo other) const {
+    return innerInfo.hasSameIsolation(other);
+  }
+
+  SWIFT_DEBUG_DUMP { innerInfo.dump(); }
+
+  void printForDiagnostics(llvm::raw_ostream &os) const {
+    innerInfo.printForDiagnostics(os);
+  }
+};
+
 } // namespace swift
 
 #endif

lib/SILOptimizer/Mandatory/TransferNonSendable.cpp

@@ -438,17 +438,17 @@ struct TransferredNonTransferrableInfo {
   ///
   /// This is equal to the merge of the IsolationRegionInfo from all elements in
   /// nonTransferrable's region when the error was diagnosed.
-  SILIsolationInfo isolationRegionInfo;
+  SILDynamicMergedIsolationInfo isolationRegionInfo;
 
-  TransferredNonTransferrableInfo(Operand *transferredOperand,
-                                  SILValue nonTransferrableValue,
-                                  SILIsolationInfo isolationRegionInfo)
+  TransferredNonTransferrableInfo(
+      Operand *transferredOperand, SILValue nonTransferrableValue,
+      SILDynamicMergedIsolationInfo isolationRegionInfo)
       : transferredOperand(transferredOperand),
         nonTransferrable(nonTransferrableValue),
         isolationRegionInfo(isolationRegionInfo) {}
-  TransferredNonTransferrableInfo(Operand *transferredOperand,
-                                  SILInstruction *nonTransferrableInst,
-                                  SILIsolationInfo isolationRegionInfo)
+  TransferredNonTransferrableInfo(
+      Operand *transferredOperand, SILInstruction *nonTransferrableInst,
+      SILDynamicMergedIsolationInfo isolationRegionInfo)
       : transferredOperand(transferredOperand),
         nonTransferrable(nonTransferrableInst),
         isolationRegionInfo(isolationRegionInfo) {}
@@ -794,7 +794,7 @@ bool UseAfterTransferDiagnosticInferrer::initForIsolatedPartialApply(
             VariableNameInferrer::inferNameAndRoot(transferOp->get())) {
       diagnosticEmitter.emitNamedIsolationCrossingDueToCapture(
           RegularLocation(std::get<0>(p).getLoc()), rootValueAndName->first,
-          state.isolationInfo, std::get<2>(p));
+          state.isolationInfo.getIsolationInfo(), std::get<2>(p));
       continue;
     }
 
@@ -975,7 +975,8 @@ void UseAfterTransferDiagnosticInferrer::infer() {
             VariableNameInferrer::inferNameAndRoot(transferOp->get())) {
       auto &state = transferringOpToStateMap.get(transferOp);
       return diagnosticEmitter.emitNamedIsolationCrossingError(
-          baseLoc, rootValueAndName->first, state.isolationInfo,
+          baseLoc, rootValueAndName->first,
+          state.isolationInfo.getIsolationInfo(),
           *sourceApply->getIsolationCrossing());
     }
 
@@ -1003,7 +1004,8 @@ void UseAfterTransferDiagnosticInferrer::infer() {
   auto captureInfo =
       autoClosureExpr->getCaptureInfo().getCaptures()[captureIndex];
   auto *captureDecl = captureInfo.getDecl();
-  AutoClosureWalker walker(*this, captureDecl, state.isolationInfo);
+  AutoClosureWalker walker(*this, captureDecl,
+                           state.isolationInfo.getIsolationInfo());
   autoClosureExpr->walk(walker);
 }
 
@@ -1109,7 +1111,7 @@ class TransferNonTransferrableDiagnosticEmitter {
   }
 
   /// Return the isolation region info for \p getNonTransferrableValue().
-  SILIsolationInfo getIsolationRegionInfo() const {
+  SILDynamicMergedIsolationInfo getIsolationRegionInfo() const {
     return info.isolationRegionInfo;
   }
 
@@ -1536,10 +1538,9 @@ struct DiagnosticEvaluator final
                                            partitionOp.getSourceInst());
   }
 
-  void
-  handleTransferNonTransferrable(const PartitionOp &partitionOp,
-                                 Element transferredVal,
-                                 SILIsolationInfo isolationRegionInfo) const {
+  void handleTransferNonTransferrable(
+      const PartitionOp &partitionOp, Element transferredVal,
+      SILDynamicMergedIsolationInfo isolationRegionInfo) const {
     LLVM_DEBUG(llvm::dbgs()
                    << "    Emitting TransferNonTransferrable Error!\n"
                    << "        ID:  %%" << transferredVal << "\n"
@@ -1556,11 +1557,10 @@ struct DiagnosticEvaluator final
         partitionOp.getSourceOp(), nonTransferrableValue, isolationRegionInfo);
   }
 
-  void
-  handleTransferNonTransferrable(const PartitionOp &partitionOp,
-                                 Element transferredVal,
-                                 Element actualNonTransferrableValue,
-                                 SILIsolationInfo isolationRegionInfo) const {
+  void handleTransferNonTransferrable(
+      const PartitionOp &partitionOp, Element transferredVal,
+      Element actualNonTransferrableValue,
+      SILDynamicMergedIsolationInfo isolationRegionInfo) const {
     LLVM_DEBUG(llvm::dbgs()
                    << "    Emitting TransferNonTransferrable Error!\n"
                    << "        ID:  %%" << transferredVal << "\n"

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -640,11 +640,12 @@ void SILIsolationInfo::print(llvm::raw_ostream &os) const {
   }
 }
 
-SILIsolationInfo SILIsolationInfo::merge(SILIsolationInfo other) const {
+SILDynamicMergedIsolationInfo
+SILIsolationInfo::merge(SILIsolationInfo other) const {
   // If we are greater than the other kind, then we are further along the
   // lattice. We ignore the change.
   if (unsigned(other.kind) < unsigned(kind))
-    return *this;
+    return {*this};
 
   // TODO: Make this failing mean that we emit an unknown SIL error instead of
   // asserting.
@@ -840,3 +841,37 @@ bool SILIsolationInfo::isNonSendableType(SILType type, SILFunction *fn) {
   // Otherwise, delegate to seeing if type conforms to the Sendable protocol.
   return !type.isSendable(fn);
 }
+
+//===----------------------------------------------------------------------===//
+//                    MARK: SILDynamicMergedIsolationInfo
+//===----------------------------------------------------------------------===//
+
+SILDynamicMergedIsolationInfo
+SILDynamicMergedIsolationInfo::merge(SILIsolationInfo other) const {
+  // If we are greater than the other kind, then we are further along the
+  // lattice. We ignore the change.
+  if (unsigned(other.getKind()) < unsigned(innerInfo.getKind()))
+    return {*this};
+
+  // TODO: Make this failing mean that we emit an unknown SIL error instead of
+  // asserting.
+  assert((!other.isActorIsolated() || !innerInfo.isActorIsolated() ||
+          innerInfo.hasSameIsolation(other)) &&
+         "Actor can only be merged with the same actor");
+
+  // If we are both disconnected and other has the unsafeNonIsolated bit set,
+  // drop that bit and return that.
+  //
+  // DISCUSSION: We do not want to preserve the unsafe non isolated bit after
+  // merging. These bits should not propagate through merging and should instead
+  // always be associated with non-merged infos.
+  //
+  // TODO: We should really bake the above into the type system by having merged
+  // and non-merged SILIsolationInfo.
+  if (other.isDisconnected() && other.isUnsafeNonIsolated()) {
+    return other.withUnsafeNonIsolated(false);
+  }
+
+  // Otherwise, just return other.
+  return other;
+}