Merge pull request #72867 from gottesmm/release/6.0-region-iso

[6.0][region-isolation] Enable region isolation when strict concurrency is enabled

Author: gottesmm

cmake/modules/AddSwiftUnittests.cmake

@@ -111,6 +111,9 @@ function(add_swift_unittest test_dirname)
     endif()
   endif()
 
+  is_build_type_with_debuginfo("${CMAKE_BUILD_TYPE}" HAS_DEBUG_INFO)
+  target_compile_options("${test_dirname}" PRIVATE $<$<BOOL:${HAS_DEBUG_INFO}>:-g>)
+
   file(RELATIVE_PATH relative_lib_path "${CMAKE_CURRENT_BINARY_DIR}" "${SWIFT_LIBRARY_OUTPUT_INTDIR}")
 
   if(SWIFT_HOST_VARIANT_SDK IN_LIST SWIFT_DARWIN_PLATFORMS)

include/swift/AST/ActorIsolation.h

@@ -111,6 +111,9 @@ class ActorIsolation {
 
   static ActorIsolation forActorInstanceSelf(ValueDecl *decl);
 
+  /// Create an ActorIsolation appropriate for a type that is self.
+  static ActorIsolation forActorInstanceSelf(NominalTypeDecl *decl);
+
   static ActorIsolation forActorInstanceParameter(NominalTypeDecl *actor,
                                                   unsigned parameterIndex) {
     return ActorIsolation(ActorInstance, actor, parameterIndex + 1);

include/swift/AST/DiagnosticsSIL.def

@@ -46,6 +46,9 @@ ERROR(bridging_objcbridgeable_broken,none,
 ERROR(sil_function_redefinition,none,
       "multiple definitions of symbol '%0'",
       (StringRef))
+NOTE(sil_function_redefinition_note,none,
+      "other definition here",
+      ())
 
 ERROR(invalid_sil_builtin,none,
       "INTERNAL ERROR: invalid use of builtin: %0",

include/swift/Option/FrontendOptions.td

@@ -1332,4 +1332,9 @@ def disable_experimental_parser_round_trip : Flag<["-"],
   "disable-experimental-parser-round-trip">,
   HelpText<"Disable round trip through the new swift parser">;
 
+def disable_strict_concurrency_region_based_isolation : Flag<["-"],
+  "disable-region-based-isolation-with-strict-concurrency">,
+  HelpText<"Disable region based isolation when running with strict concurrency enabled. Only enabled with asserts">,
+  Flags<[HelpHidden]>;
+
 } // end let Flags = [FrontendOption, NoDriverOption, HelpHidden]

include/swift/SIL/SILFunction.h

@@ -348,7 +348,7 @@ class SILFunction
   unsigned BlockListChangeIdx = 0;
 
   /// The isolation of this function.
-  std::optional<ActorIsolation> actorIsolation;
+  ActorIsolation actorIsolation = ActorIsolation::forUnspecified();
 
   /// The function's bare attribute. Bare means that the function is SIL-only
   /// and does not require debug info.
@@ -1374,9 +1374,7 @@ class SILFunction
     actorIsolation = newActorIsolation;
   }
 
-  std::optional<ActorIsolation> getActorIsolation() const {
-    return actorIsolation;
-  }
+  ActorIsolation getActorIsolation() const { return actorIsolation; }
 
   //===--------------------------------------------------------------------===//
   // Block List Access

include/swift/SILOptimizer/Analysis/RegionAnalysis.h

@@ -162,7 +162,7 @@ class regionanalysisimpl::TrackableValueState {
   }
 
   ActorIsolation getActorIsolation() const {
-    return regionInfo.getActorIsolation().value();
+    return regionInfo.getActorIsolation();
   }
 
   void mergeIsolationRegionInfo(SILIsolationInfo newRegionInfo) {

include/swift/SILOptimizer/Utils/PartitionUtils.h

@@ -110,20 +110,19 @@ class SILIsolationInfo {
   // clang-format off
   std::variant<
     // Used for actor isolated when we have ActorIsolation info from the AST.
-    std::optional<ActorIsolation>,
-    // Used for actor isolation when we infer the actor at the SIL level.
-    NominalTypeDecl *,
+    ActorIsolation,
     // The task isolated parameter when we find a task isolated value.
     SILValue
   > data;
   // clang-format on
 
-  SILIsolationInfo(Kind kind, std::optional<ActorIsolation> actorIsolation)
-      : kind(kind), data(actorIsolation) {}
-  SILIsolationInfo(Kind kind, NominalTypeDecl *decl) : kind(kind), data(decl) {}
+  SILIsolationInfo(ActorIsolation actorIsolation)
+      : kind(Actor), data(actorIsolation) {}
 
   SILIsolationInfo(Kind kind, SILValue value) : kind(kind), data(value) {}
 
+  SILIsolationInfo(Kind kind) : kind(kind), data() {}
+
 public:
   SILIsolationInfo() : kind(Kind::Unknown), data() {}
 
@@ -146,18 +145,11 @@ class SILIsolationInfo {
 
   void printForDiagnostics(llvm::raw_ostream &os) const;
 
-  std::optional<ActorIsolation> getActorIsolation() const {
+  ActorIsolation getActorIsolation() const {
     assert(kind == Actor);
-    assert(std::holds_alternative<std::optional<ActorIsolation>>(data) &&
+    assert(std::holds_alternative<ActorIsolation>(data) &&
            "Doesn't have an actor isolation?!");
-    return std::get<std::optional<ActorIsolation>>(data);
-  }
-
-  NominalTypeDecl *getActorInstance() const {
-    assert(kind == Actor);
-    assert(std::holds_alternative<NominalTypeDecl *>(data) &&
-           "Doesn't have an actor instance?!");
-    return std::get<NominalTypeDecl *>(data);
+    return std::get<ActorIsolation>(data);
   }
 
   SILValue getTaskIsolatedValue() const {
@@ -167,45 +159,35 @@ class SILIsolationInfo {
     return std::get<SILValue>(data);
   }
 
-  bool hasActorIsolation() const {
-    return kind == Actor &&
-           std::holds_alternative<std::optional<ActorIsolation>>(data);
-  }
-
-  bool hasActorInstance() const {
-    return kind == Actor && std::holds_alternative<NominalTypeDecl *>(data);
-  }
+  bool hasActorIsolation() const { return kind == Actor; }
 
   bool hasTaskIsolatedValue() const {
     return kind == Task && std::holds_alternative<SILValue>(data);
   }
 
-  /// If we actually have an actor decl, return that. Otherwise, see if we have
-  /// an actor isolation if we can find one in there. Returns nullptr if we
-  /// fail.
-  NominalTypeDecl *tryInferActorDecl() const;
-
   [[nodiscard]] SILIsolationInfo merge(SILIsolationInfo other) const;
 
   SILIsolationInfo withActorIsolated(ActorIsolation isolation) {
     return SILIsolationInfo::getActorIsolated(isolation);
   }
 
-  static SILIsolationInfo getDisconnected() { return {Kind::Disconnected, {}}; }
+  static SILIsolationInfo getDisconnected() { return {Kind::Disconnected}; }
 
   static SILIsolationInfo getActorIsolated(ActorIsolation actorIsolation) {
-    return {Kind::Actor, actorIsolation};
+    return {actorIsolation};
   }
 
-  /// Sometimes we may have something that is actor isolated or that comes from
-  /// a type. First try getActorIsolation and otherwise, just use the type.
-  static SILIsolationInfo getActorIsolated(NominalTypeDecl *nomDecl) {
-    auto actorIsolation = swift::getActorIsolation(nomDecl);
-    if (actorIsolation.isActorIsolated())
-      return getActorIsolated(actorIsolation);
-    if (nomDecl->isActor())
-      return {Kind::Actor, nomDecl};
-    return SILIsolationInfo();
+  static SILIsolationInfo getActorIsolated(NominalTypeDecl *typeDecl) {
+    if (typeDecl->isActor())
+      return {ActorIsolation::forActorInstanceSelf(typeDecl)};
+    auto isolation = swift::getActorIsolation(typeDecl);
+    if (isolation.isGlobalActor())
+      return {isolation};
+    return {};
+  }
+
+  static SILIsolationInfo getGlobalActorIsolated(Type globalActorType) {
+    return getActorIsolated(ActorIsolation::forGlobalActor(globalActorType));
   }
 
   static SILIsolationInfo getTaskIsolated(SILValue value) {
@@ -901,8 +883,8 @@ struct PartitionOpEvaluator {
       // our transferring operand. If so, we can squelch this.
       if (auto functionIsolation =
               transferringOp->getUser()->getFunction()->getActorIsolation()) {
-        if (functionIsolation->isActorIsolated() &&
-            SILIsolationInfo::getActorIsolated(*functionIsolation) ==
+        if (functionIsolation.isActorIsolated() &&
+            SILIsolationInfo::getActorIsolated(functionIsolation) ==
                 SILIsolationInfo::get(transferringOp->getUser()))
           return;
       }

lib/AST/Decl.cpp

@@ -11387,6 +11387,10 @@ ActorIsolation::forActorInstanceSelf(ValueDecl *decl) {
   return ActorIsolation(ActorInstance, dc->getSelfNominalTypeDecl(), 0);
 }
 
+ActorIsolation ActorIsolation::forActorInstanceSelf(NominalTypeDecl *selfDecl) {
+  return ActorIsolation(ActorInstance, selfDecl, 0);
+}
+
 NominalTypeDecl *ActorIsolation::getActor() const {
   assert(getKind() == ActorInstance ||
          getKind() == GlobalActor);

lib/DriverTool/sil_opt_main.cpp

@@ -534,6 +534,11 @@ struct SILOptOptions {
   llvm::cl::list<std::string> ClangXCC = llvm::cl::list<std::string>(
       "Xcc",
       llvm::cl::desc("option to pass to clang"));
+
+  llvm::cl::opt<bool> DisableRegionBasedIsolationWithStrictConcurrency =
+      llvm::cl::opt<bool>(
+          "disable-region-based-isolation-with-strict-concurrency",
+          llvm::cl::init(false));
 };
 
 /// Regular expression corresponding to the value given in one of the
@@ -698,9 +703,14 @@ int sil_opt_main(ArrayRef<const char *> argv, void *MainAddr) {
 
   Invocation.getLangOptions().UnavailableDeclOptimizationMode =
       options.UnavailableDeclOptimization;
-  if (options.StrictConcurrencyLevel.hasArgStr())
+  if (options.StrictConcurrencyLevel.hasArgStr()) {
     Invocation.getLangOptions().StrictConcurrencyLevel =
         options.StrictConcurrencyLevel;
+    if (options.StrictConcurrencyLevel == StrictConcurrency::Complete &&
+        !options.DisableRegionBasedIsolationWithStrictConcurrency) {
+      Invocation.getLangOptions().enableFeature(Feature::RegionBasedIsolation);
+    }
+  }
 
   Invocation.getDiagnosticOptions().VerifyMode =
     options.VerifyMode ? DiagnosticOptions::Verify : DiagnosticOptions::NoVerify;

lib/Frontend/CompilerInvocation.cpp

@@ -1060,6 +1060,16 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   if (Opts.StrictConcurrencyLevel == StrictConcurrency::Complete) {
     Opts.enableFeature(Feature::IsolatedDefaultValues);
     Opts.enableFeature(Feature::GlobalConcurrency);
+
+    // If asserts are enabled, allow for region based isolation to be disabled
+    // with a flag. This is intended only to be used with tests.
+    bool enableRegionIsolation = true;
+#ifndef NDEBUG
+    enableRegionIsolation =
+        !Args.hasArg(OPT_disable_strict_concurrency_region_based_isolation);
+#endif
+    if (enableRegionIsolation)
+      Opts.enableFeature(Feature::RegionBasedIsolation);
   }
 
   Opts.WarnImplicitOverrides =