[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.

(cherry picked from commit 741244e)

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;
+}