[region-isolation] Initialize TrackableValueState's regionInfo with a .none instead of a disconnected region.

The design change here is that instead of just initializing the regionInfo with
disconnected, we set it as .none and if we see .none, just return a newly
construct disconnected isolation region info when getIsolationRegionInfo() is
called.

This enables us to provide a setIsolationRegionInfo() helper for
RegionAnalysisValueMap::getTrackableValue that does not perform a merge. This is
important since for nonisolated(unsafe), we want to not have nonisolated(unsafe)
propagate through merging. So if we use merging to initialize the internal
regionInfo state of a SILIsolationInfo, we will never have a SILIsolationInfo
with that bit set since it will be lost in the merge. So we need some sort of
other assignment operator. Noting that we should only compute a value's
SILIsolationInfo once in RegionAnalysisValueMap before we cache it in the map,
it made sense to just represent it as an optional that way we can guarantee that
the regionInfo is only ever set exactly once by that routine.

(cherry picked from commit 89a2cfc)

Author: gottesmm

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -24,6 +24,7 @@
 namespace swift {
 
 class RegionAnalysisFunctionInfo;
+class RegionAnalysisValueMap;
 
 namespace regionanalysisimpl {
 
@@ -121,10 +122,11 @@ using TrackedValueFlagSet = OptionSet<TrackableValueFlag>;
 } // namespace regionanalysisimpl
 
 class regionanalysisimpl::TrackableValueState {
+  friend RegionAnalysisValueMap;
+
   unsigned id;
   TrackedValueFlagSet flagSet = {TrackableValueFlag::isMayAlias};
-  SILIsolationInfo regionInfo =
-      SILIsolationInfo::getDisconnected(false /*nonisolated(unsafe)*/);
+  std::optional<SILIsolationInfo> regionInfo = {};
 
 public:
   TrackableValueState(unsigned newID) : id(newID) {}
@@ -141,20 +143,26 @@ class regionanalysisimpl::TrackableValueState {
 
   bool isNonSendable() const { return !isSendable(); }
 
+  SILIsolationInfo getIsolationRegionInfo() const {
+    if (!regionInfo) {
+      return SILIsolationInfo::getDisconnected(false);
+    }
+
+    return *regionInfo;
+  }
+
   SILIsolationInfo::Kind getIsolationRegionInfoKind() const {
-    return regionInfo.getKind();
+    return getIsolationRegionInfo().getKind();
   }
 
   ActorIsolation getActorIsolation() const {
-    return regionInfo.getActorIsolation();
+    return getIsolationRegionInfo().getActorIsolation();
   }
 
   void mergeIsolationRegionInfo(SILIsolationInfo newRegionInfo) {
-    regionInfo = regionInfo.merge(newRegionInfo);
+    regionInfo = getIsolationRegionInfo().merge(newRegionInfo);
   }
 
-  SILIsolationInfo getIsolationRegionInfo() const { return regionInfo; }
-
   Element getID() const { return Element(id); }
 
   void addFlag(TrackableValueFlag flag) { flagSet |= flag; }
@@ -166,11 +174,22 @@ class regionanalysisimpl::TrackableValueState {
        << "][is_no_alias: " << (isNoAlias() ? "yes" : "no")
        << "][is_sendable: " << (isSendable() ? "yes" : "no")
        << "][region_value_kind: ";
-    getIsolationRegionInfo().printForDiagnostics(os);
+    getIsolationRegionInfo().printForOneLineLogging(os);
     os << "].";
   }
 
   SWIFT_DEBUG_DUMP { print(llvm::dbgs()); }
+
+private:
+  bool hasIsolationRegionInfo() const { return bool(regionInfo); }
+
+  /// Set the isolation region info for this TrackableValueState. Private so it
+  /// can only be used by RegionAnalysisValueMap::getTrackableValue.
+  void setIsolationRegionInfo(SILIsolationInfo newRegionInfo) {
+    assert(!regionInfo.has_value() &&
+           "Can only call setIsolationRegionInfo once!\n");
+    regionInfo = newRegionInfo;
+  }
 };
 
 /// The representative value of the equivalence class that makes up a tracked

include/swift/SILOptimizer/Utils/SILIsolationInfo.h

@@ -187,6 +187,12 @@ class SILIsolationInfo {
 
   void print(llvm::raw_ostream &os) const;
 
+  /// Print a textual representation of the text info that is meant to be
+  /// included in other logging output for types that compose with
+  /// SILIsolationInfo. As a result, we only print state that can fit on
+  /// one line.
+  void printForOneLineLogging(llvm::raw_ostream &os) const;
+
   SWIFT_DEBUG_DUMP {
     print(llvm::dbgs());
     llvm::dbgs() << '\n';

lib/SILOptimizer/Analysis/RegionAnalysis.cpp

@@ -3255,7 +3255,27 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
 
   // Otherwise, we need to compute our flags.
 
-  // First for addresses.
+  // Treat function ref and class method as either actor isolated or
+  // sendable. Formally they are non-Sendable, so we do the check before we
+  // check the oracle.
+  if (isa<FunctionRefInst, ClassMethodInst>(value)) {
+    if (auto isolation = SILIsolationInfo::get(value)) {
+      iter.first->getSecond().setIsolationRegionInfo(isolation);
+      return {iter.first->first, iter.first->second};
+    }
+
+    iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
+    return {iter.first->first, iter.first->second};
+  }
+
+  // Then check our oracle to see if the value is actually sendable. If we have
+  // a Sendable value, just return early.
+  if (!SILIsolationInfo::isNonSendableType(value->getType(), fn)) {
+    iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
+    return {iter.first->first, iter.first->second};
+  }
+
+  // Ok, at this point we have a non-Sendable value. First process addresses.
   if (value->getType().isAddress()) {
     // If we were able to find this was actor isolated from finding our
     // underlying object, use that. It is never wrong.
@@ -3270,7 +3290,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
       }
 
       if (isolation) {
-        iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+        iter.first->getSecond().setIsolationRegionInfo(isolation);
       }
     }
 
@@ -3284,32 +3304,14 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
       }
 
       if (auto isolation = SILIsolationInfo::get(storage.base)) {
-        iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+        iter.first->getSecond().setIsolationRegionInfo(isolation);
       }
     }
   }
 
-  // Treat function ref and class method as either actor isolated or
-  // sendable. Formally they are non-Sendable, so we do the check before we
-  // check the oracle.
-  if (isa<FunctionRefInst, ClassMethodInst>(value)) {
-    if (auto isolation = SILIsolationInfo::get(value)) {
-      iter.first->getSecond().mergeIsolationRegionInfo(isolation);
-      return {iter.first->first, iter.first->second};
-    }
-
-    iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
-    return {iter.first->first, iter.first->second};
-  }
-
-  // Otherwise refer to the oracle. If we have a Sendable value, just return.
-  if (!SILIsolationInfo::isNonSendableType(value->getType(), fn)) {
-    iter.first->getSecond().addFlag(TrackableValueFlag::isSendable);
-    return {iter.first->first, iter.first->second};
-  }
-
-  // Check if our base is a ref_element_addr from an actor. In such a case,
-  // mark this value as actor derived.
+  // Check if we have a load or load_borrow from an address. In that case, we
+  // want to look through the load and find a better root from the address we
+  // loaded from.
   if (isa<LoadInst, LoadBorrowInst>(iter.first->first.getValue())) {
     auto *svi = cast<SingleValueInstruction>(iter.first->first.getValue());
 
@@ -3320,7 +3322,7 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     // everywhere. Just haven't done it due to possible perturbations.
     auto parentAddrInfo = getUnderlyingTrackedValue(svi);
     if (parentAddrInfo.actorIsolation) {
-      iter.first->getSecond().mergeIsolationRegionInfo(
+      iter.first->getSecond().setIsolationRegionInfo(
           SILIsolationInfo::getActorInstanceIsolated(
               svi, parentAddrInfo.value,
               parentAddrInfo.actorIsolation->getActor()));
@@ -3329,16 +3331,20 @@ TrackableValue RegionAnalysisValueMap::getTrackableValue(
     auto storage = AccessStorageWithBase::compute(svi->getOperand(0));
     if (storage.storage) {
       if (auto isolation = SILIsolationInfo::get(storage.base)) {
-        iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+        iter.first->getSecond().setIsolationRegionInfo(isolation);
       }
     }
 
     return {iter.first->first, iter.first->second};
   }
 
-  // Ok, we have a non-Sendable type, attempt to infer its isolation.
-  if (auto isolation = SILIsolationInfo::get(iter.first->first.getValue())) {
-    iter.first->getSecond().mergeIsolationRegionInfo(isolation);
+  // Ok, we have a non-Sendable type, see if we do not have any isolation
+  // yet. If we don't, attempt to infer its isolation.
+  if (!iter.first->getSecond().hasIsolationRegionInfo()) {
+    if (auto isolation = SILIsolationInfo::get(iter.first->first.getValue())) {
+      iter.first->getSecond().setIsolationRegionInfo(isolation);
+      return {iter.first->first, iter.first->second};
+    }
   }
 
   return {iter.first->first, iter.first->second};

lib/SILOptimizer/Utils/SILIsolationInfo.cpp

@@ -629,6 +629,49 @@ void SILIsolationInfo::printForDiagnostics(llvm::raw_ostream &os) const {
   }
 }
 
+void SILIsolationInfo::printForOneLineLogging(llvm::raw_ostream &os) const {
+  switch (Kind(*this)) {
+  case Unknown:
+    os << "unknown";
+    return;
+  case Disconnected:
+    os << "disconnected";
+    if (unsafeNonIsolated) {
+      os << ": nonisolated(unsafe)";
+    }
+    return;
+  case Actor:
+    if (auto instance = getActorInstance()) {
+      switch (instance.getKind()) {
+      case ActorInstance::Kind::Value: {
+        SILValue value = instance.getValue();
+        if (auto name = VariableNameInferrer::inferName(value)) {
+          os << "'" << *name << "'-isolated";
+          return;
+        }
+        break;
+      }
+      case ActorInstance::Kind::ActorAccessorInit:
+        os << "'self'-isolated";
+        return;
+      }
+    }
+
+    if (getActorIsolation().getKind() == ActorIsolation::ActorInstance) {
+      if (auto *vd = getActorIsolation().getActorInstance()) {
+        os << "'" << vd->getBaseIdentifier() << "'-isolated";
+        return;
+      }
+    }
+
+    getActorIsolation().printForDiagnostics(os);
+    return;
+  case Task:
+    os << "task-isolated";
+    return;
+  }
+}
+
 // Check if the passed in type is NonSendable.
 //
 // NOTE: We special case RawPointer and NativeObject to ensure they are