Merge pull request #71663 from apple/es-bypass-opt

Move in-package resilience bypassing logic to a resilience check with accessing module

Author: elsh

include/swift/AST/Decl.h

@@ -2930,6 +2930,11 @@ class ValueDecl : public Decl {
   /// if the base declaration is \c open, the override might have to be too.
   bool hasOpenAccess(const DeclContext *useDC) const;
 
+  /// True if opted in for bypassing resilience within a package. Allowed only on
+  /// access from the \p accessingModule to a package decl in the defining
+  /// binary (not interface) module within the same package.
+  bool bypassResilienceInPackage(ModuleDecl *accessingModule) const;
+
   /// FIXME: This is deprecated.
   bool isRecursiveValidation() const;
 

include/swift/AST/DeclContext.h

@@ -530,13 +530,6 @@ class alignas(1 << DeclContextAlignInBits) DeclContext
   LLVM_READONLY
   PackageUnit *getPackageContext(bool lookupIfNotCurrent = false) const;
 
-  /// True if resilience checks can be bypassed within a package.
-  /// \p isForPackageDecl Bypassing only applies to package types
-  /// (possibly also public types later) if opted-in, client and defining module
-  /// are in the same package, and the defining module is a binary module.
-  LLVM_READONLY
-  bool bypassResilienceInPackage(bool isForPackageDecl) const;
-
   /// Returns the module context that contains this context.
   LLVM_READONLY
   ModuleDecl *getParentModule() const;

include/swift/AST/Module.h

@@ -482,7 +482,9 @@ class ModuleDecl
   }
 
   bool inSamePackage(ModuleDecl *other) {
-    return !getPackageName().empty() && getPackageName() == other->getPackageName();
+    return other != nullptr &&
+           !getPackageName().empty() &&
+           getPackageName() == other->getPackageName();
   }
 
   /// Get the package associated with this module

include/swift/Option/Options.td

@@ -516,7 +516,7 @@ def unavailable_decl_optimization_EQ : Joined<["-"], "unavailable-decl-optimizat
            "value may be 'none' (no optimization) or 'complete' (code is not "
            "generated at all unavailable declarations)">;
 
-def package_bypass_resilience_optimization : Flag<["-"], "package-bypass-resilience-optimization">,
+def experimental_package_bypass_resilience : Flag<["-"], "experimental-package-bypass-resilience">,
   Flags<[FrontendOption]>,
   HelpText<"Enable optimization to bypass resilience within a package">;
 

lib/AST/Decl.cpp

@@ -2968,13 +2968,7 @@ bool AbstractStorageDecl::isResilient() const {
   if (!accessScope.isPublicOrPackage())
     return false;
 
-  if (!getModuleContext()->isResilient())
-    return false;
-
-  // Allows bypassing resilience checks for package decls
-  // at use site within a package if opted in, whether the
-  // loaded module was built resiliently or not.
-  return !getDeclContext()->bypassResilienceInPackage(accessScope.isPackage());
+  return getModuleContext()->isResilient();
 }
 
 bool AbstractStorageDecl::isResilient(ModuleDecl *M,
@@ -2983,7 +2977,12 @@ bool AbstractStorageDecl::isResilient(ModuleDecl *M,
   case ResilienceExpansion::Minimal:
     return isResilient();
   case ResilienceExpansion::Maximal:
-    return M != getModuleContext() && isResilient();
+    if (M == getModuleContext())
+      return false;
+    // Non-resilient if bypass optimization in package is enabled
+    if (bypassResilienceInPackage(M))
+      return false;
+    return isResilient();
   }
   llvm_unreachable("bad resilience expansion");
 }
@@ -4205,6 +4204,14 @@ bool ValueDecl::hasOpenAccess(const DeclContext *useDC) const {
   return access == AccessLevel::Open;
 }
 
+bool ValueDecl::bypassResilienceInPackage(ModuleDecl *accessingModule) const {
+  return getASTContext().LangOpts.EnableBypassResilienceInPackage &&
+          getModuleContext()->inSamePackage(accessingModule) &&
+          !getModuleContext()->isBuiltFromInterface() &&
+          getFormalAccessScope(/*useDC=*/nullptr,
+                               /*treatUsableFromInlineAsPublic=*/true).isPackage();
+}
+
 /// Given the formal access level for using \p VD, compute the scope where
 /// \p VD may be accessed, taking \@usableFromInline, \@testable imports,
 /// \@_spi imports, and enclosing access levels into account.
@@ -5045,14 +5052,7 @@ bool NominalTypeDecl::isFormallyResilient() const {
 bool NominalTypeDecl::isResilient() const {
   if (!isFormallyResilient())
     return false;
-  if (!getModuleContext()->isResilient())
-    return false;
-  // Allows bypassing resilience checks for package decls
-  // at use site within a package if opted in, whether the
-  // loaded module was built resiliently or not.
-  auto accessScope = getFormalAccessScope(/*useDC=*/nullptr,
-                                          /*treatUsableFromInlineAsPublic=*/true);
-  return !getDeclContext()->bypassResilienceInPackage(accessScope.isPackage());
+  return getModuleContext()->isResilient();
 }
 
 DestructorDecl *NominalTypeDecl::getValueTypeDestructor() {
@@ -5083,9 +5083,12 @@ bool NominalTypeDecl::isResilient(ModuleDecl *M,
   case ResilienceExpansion::Maximal:
     // We can access declarations from the same module
     // non-resiliently in a maximal context.
-    if (M == getModuleContext()) {
+    if (M == getModuleContext())
       return false;
-    }
+    // Non-resilient if bypass optimization in package is enabled
+    if (bypassResilienceInPackage(M))
+      return false;
+
     // If a protocol is originally declared in the current module, then we
     // directly expose protocol witness tables and their contents for any
     // conformances in the same module as symbols. If the protocol later
@@ -6376,7 +6379,7 @@ bool EnumDecl::isFormallyExhaustive(const DeclContext *useDC) const {
   // package enum is optimized with bypassing resilience checks.
   if (!accessScope.isPublicOrPackage())
     return true;
-  if (useDC && useDC->bypassResilienceInPackage(accessScope.isPackage()))
+  if (useDC && bypassResilienceInPackage(useDC->getParentModule()))
     return true;
 
   // All other checks are use-site specific; with no further information, the

lib/AST/DeclContext.cpp

@@ -300,17 +300,6 @@ PackageUnit *DeclContext::getPackageContext(bool lookupIfNotCurrent) const {
   return nullptr;
 }
 
-bool DeclContext::bypassResilienceInPackage(bool isForPackageDecl) const {
-  // Bypassing resilience checks only applies to package types (and possibly
-  // public types in the same package in the future). Allowed only if opted-in
-  // for bypassing optimization, client and defining module are in the same
-  // package, and defining module is a binary module.
-  return isForPackageDecl &&
-         getASTContext().LangOpts.EnableBypassResilienceInPackage &&
-         getParentModule()->inSamePackage(getASTContext().MainModule) &&
-         !getParentModule()->isBuiltFromInterface();
-}
-
 ModuleDecl *DeclContext::getParentModule() const {
   // If the current context is PackageUnit, return the module
   // decl context pointing to the current context. This check

lib/Driver/ToolChains.cpp

@@ -299,7 +299,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
   inputArgs.AddLastArg(arguments, options::OPT_Rpass_missed_EQ);
   inputArgs.AddLastArg(arguments, options::OPT_suppress_warnings);
   inputArgs.AddLastArg(arguments, options::OPT_suppress_remarks);
-  inputArgs.AddLastArg(arguments, options::OPT_package_bypass_resilience_optimization);
+  inputArgs.AddLastArg(arguments, options::OPT_experimental_package_bypass_resilience);
   inputArgs.AddLastArg(arguments, options::OPT_profile_generate);
   inputArgs.AddLastArg(arguments, options::OPT_profile_use);
   inputArgs.AddLastArg(arguments, options::OPT_profile_coverage_mapping);

lib/Frontend/CompilerInvocation.cpp

@@ -671,7 +671,7 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   Opts.EnablePackageInterfaceLoad = Args.hasArg(OPT_experimental_package_interface_load) ||
                                     ::getenv("SWIFT_ENABLE_PACKAGE_INTERFACE_LOAD");
 
-  Opts.EnableBypassResilienceInPackage = Args.hasArg(OPT_package_bypass_resilience_optimization);
+  Opts.EnableBypassResilienceInPackage = Args.hasArg(OPT_experimental_package_bypass_resilience);
 
   Opts.DisableAvailabilityChecking |=
       Args.hasArg(OPT_disable_availability_checking);

test/IRGen/package_resilience.swift

@@ -2,32 +2,37 @@
 // Unlike its counterparts in the other *_resilience.swift files, the goal is
 // for the package's component modules to all be considered within the same
 // resilience domain. This file ensures that we use direct access to package
-// decls at use site as much as possible with -package-bypass-resilience-optimization.
+// decls at use site as much as possible with -experimental-package-bypass-resilience.
 //
 
 // RUN: %empty-directory(%t)
 // RUN: %{python} %utils/chex.py < %s > %t/package_resilience.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_struct.swiftmodule -module-name=resilient_struct %S/Inputs/package_types/resilient_struct.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_enum.swiftmodule -module-name=resilient_enum -I %t %S/Inputs/package_types/resilient_enum.swift
 // RUN: %target-swift-frontend -package-name MyPkg -emit-module -enable-library-evolution -emit-module-path=%t/resilient_class.swiftmodule -module-name=resilient_class -I %t %S/Inputs/package_types/resilient_class.swift
-// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -enable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-objc,CHECK-objc%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-%target-import-type-objc-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=7 -D#MDSIZE32=52 -D#MDSIZE64=80 -D#WORDSIZE=%target-alignment
-// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -disable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-native,CHECK-native%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-native-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=4 -D#MDSIZE32=40 -D#MDSIZE64=56 -D#WORDSIZE=%target-alignment
-// RUN: %target-swift-frontend -package-name MyPkg -package-bypass-resilience-optimization -I %t -emit-ir -enable-library-evolution -O %t/package_resilience.swift -package-name MyPkg
+
+// RUN: %target-swift-frontend -package-name MyPkg -module-name=package_resilience -experimental-package-bypass-resilience -enable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-objc,CHECK-objc%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-%target-import-type-objc-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=7 -D#MDSIZE32=52 -D#MDSIZE64=80 -D#WORDSIZE=%target-alignment
+
+// RUN: %target-swift-frontend -package-name MyPkg -module-name=package_resilience -experimental-package-bypass-resilience -disable-objc-interop -I %t -emit-ir -enable-library-evolution %t/package_resilience.swift | %FileCheck %t/package_resilience.swift --check-prefixes=CHECK,CHECK-native,CHECK-native%target-ptrsize,CHECK-%target-ptrsize,CHECK-%target-cpu,CHECK-native-STABLE-ABI-%target-mandates-stable-abi,CHECK-%target-sdk-name -DINT=i%target-ptrsize -D#MDWORDS=4 -D#MDSIZE32=40 -D#MDSIZE64=56 -D#WORDSIZE=%target-alignment
+
+// RUN: %target-swift-frontend -package-name MyPkg -module-name=package_resilience -experimental-package-bypass-resilience -I %t -emit-ir -enable-library-evolution -O %t/package_resilience.swift -package-name MyPkg
+
 // REQUIRES: objc_codegen
 // REQUIRES: OS=macosx || OS=ios || OS=tvos || OS=watchos
 // REQUIRES: CPU=x86_64 || CPU=arm64
 
-// CHECK: @"$s18package_resilience26ClassWithResilientPropertyC1p16resilient_struct5PointVvpWvd" = constant [[INT]] {{8|16}}, align [[#WORDSIZE]]
-// CHECK: @"$s18package_resilience26ClassWithResilientPropertyC1s16resilient_struct4SizeVvpWvd" = constant [[INT]] {{32|16}}, align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience26ClassWithResilientPropertyC1p16resilient_struct5PointVvpWvd" = hidden constant [[INT]] {{8|16}}, align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience26ClassWithResilientPropertyC1s16resilient_struct4SizeVvpWvd" = hidden constant [[INT]] {{32|16}}, align [[#WORDSIZE]]
 
-// CHECK: @"$s18package_resilience33ClassWithResilientlySizedPropertyC1r16resilient_struct9RectangleVvpWvd" = constant [[INT]] {{8|16}}, align [[#WORDSIZE]]
-// CHECK: @"$s18package_resilience33ClassWithResilientlySizedPropertyC5colors5Int32VvpWvd" = constant [[INT]] 56, align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience33ClassWithResilientlySizedPropertyC1r16resilient_struct9RectangleVvpWvd" = hidden constant [[INT]] {{8|16}}, align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience33ClassWithResilientlySizedPropertyC5colors5Int32VvpWvd" = hidden constant [[INT]] 56, align [[#WORDSIZE]]
 
+// class metadata base offset for package_resilience.MyResilientParent
 // CHECK: @"$s18package_resilience17MyResilientParentCMo" = constant [[BOUNDS:{ (i32|i64), i32, i32 }]]
 // CHECK-32-SAME: { [[INT]] [[#MDSIZE32]], i32 3, i32 [[#MDWORDS + 6 + 2]] }, align [[#WORDSIZE]]
 // CHECK-64-SAME: { [[INT]] [[#MDSIZE64]], i32 3, i32 [[#MDWORDS + 3 + 2]] }, align [[#WORDSIZE]]
 
-// CHECK: @"$s18package_resilience16MyResilientChildC5fields5Int32VvpWvd" = constant [[INT]] [[#WORDSIZE + WORDSIZE + 4]], align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience16MyResilientChildC5fields5Int32VvpWvd" = hidden constant [[INT]] [[#WORDSIZE + WORDSIZE + 4]], align [[#WORDSIZE]]
 
 // CHECK: @"$s18package_resilience16MyResilientChildCMo" = {{(protected )?}}{{(dllexport )?}}constant [[BOUNDS]]
 // CHECK-32-SAME: { [[INT]] [[#MDSIZE32 + WORDSIZE + WORDSIZE]], i32 3, i32 [[#MDWORDS + 6 + 3]] }
@@ -41,17 +46,16 @@
 // CHECK-32-SAME: { [[INT]] [[#MDSIZE32 + WORDSIZE + WORDSIZE + WORDSIZE]], i32 3, i32 [[#MDWORDS + 6 + 5]] }
 // CHECK-64-SAME: { [[INT]] [[#MDSIZE64 + WORDSIZE + WORDSIZE + WORDSIZE]], i32 3, i32 [[#MDWORDS + 3 + 5]] }
 
-// CHECK: @"$s18package_resilience27ClassWithEmptyThenResilientC5emptyAA0E0VvpWvd" = constant [[INT]] 0,
-// CHECK: @"$s18package_resilience27ClassWithEmptyThenResilientC9resilient0H7_struct0G3IntVvpWvd" = constant [[INT]] [[#WORDSIZE + WORDSIZE]], align [[#WORDSIZE]]
-// CHECK: @"$s18package_resilience27ClassWithResilientThenEmptyC9resilient0H7_struct0E3IntVvpWvd" = constant [[INT]] [[#WORDSIZE + WORDSIZE]], align [[#WORDSIZE]]
-// CHECK: @"$s18package_resilience27ClassWithResilientThenEmptyC5emptyAA0G0VvpWvd" = constant [[INT]] 0,
+// CHECK: @"$s18package_resilience27ClassWithEmptyThenResilientC5emptyAA0E0VvpWvd" = hidden constant [[INT]] 0,
+// CHECK: @"$s18package_resilience27ClassWithEmptyThenResilientC9resilient0H7_struct0G3IntVvpWvd" = hidden constant [[INT]] [[#WORDSIZE + WORDSIZE]], align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience27ClassWithResilientThenEmptyC9resilient0H7_struct0E3IntVvpWvd" = hidden constant [[INT]] [[#WORDSIZE + WORDSIZE]], align [[#WORDSIZE]]
+// CHECK: @"$s18package_resilience27ClassWithResilientThenEmptyC5emptyAA0G0VvpWvd" = hidden constant [[INT]] 0,
 
 import resilient_class
 import resilient_struct
 import resilient_enum
 
 // Concrete class with resilient stored property
-
 package class ClassWithResilientProperty {
   package let p: Point
   package let s: Size
@@ -230,15 +234,17 @@ public func memoryLayoutDotAlignmentWithResilientStruct() -> Int {
   return MemoryLayout<Size>.alignment
 }
 
-// CHECK: define{{( dllexport)?}}{{( protected)?}} swiftcc [[BOUNDS:{ (i32|i64), (i32|i64), i8 }]] @"$s18package_resilience31constructResilientEnumNoPayload14resilient_enum6MediumOyF"
+// CHECK: define{{( dllexport)?}}{{( protected)?}} swiftcc void  @"$s18package_resilience31constructResilientEnumNoPayload14resilient_enum6MediumOyF"
 package func constructResilientEnumNoPayload() -> Medium {
-  // CHECK: ret [[BOUNDS]] { {{i32|i64}} 0, {{i32|i64}} 0, i8 2 }
+  // CHECK: [[FIELD_PTR:%.*]] = getelementptr inbounds %T14resilient_enum6MediumO, ptr %0, i32 0, i32 1
+  // CHECK: ret void
   return Medium.Paper
 }
 
-// CHECK: define{{( dllexport)?}}{{( protected)?}} swiftcc [[BOUNDS:{ (i32|i64), (i32|i64), i8 }]] @"$s18package_resilience39constructExhaustiveWithResilientMembers14resilient_enum11SimpleShapeOyF"() #0 {
+// CHECK: define{{( dllexport)?}}{{( protected)?}} swiftcc void @"$s18package_resilience39constructExhaustiveWithResilientMembers14resilient_enum11SimpleShapeOyF"
 package func constructExhaustiveWithResilientMembers() -> SimpleShape {
-  // CHECK: ret [[BOUNDS]] { {{i32|i64}} 0, {{i32|i64}} 0, i8 1 }
+  // CHECK: [[FIELD_PTR:%.*]] = getelementptr inbounds %T14resilient_enum11SimpleShapeO, ptr %0, i32 0, i32 1
+  // CHECK: ret void
   return .KleinBottle
 }
 

test/SILGen/package_bypass_resilience.swift

@@ -0,0 +1,45 @@
+// RUN: %empty-directory(%t)
+// RUN: split-file %s %t
+
+// RUN: %target-swift-frontend -emit-module %t/Utils.swift \
+// RUN:   -module-name Utils -swift-version 5 -I %t \
+// RUN:   -package-name mypkg \
+// RUN:   -enable-library-evolution \
+// RUN:   -emit-module -emit-module-path %t/Utils.swiftmodule
+
+// RUN: %target-swift-frontend -emit-silgen %t/Client.swift -I %t -module-name Client -package-name mypkg | %FileCheck %s --check-prefixes=CHECK,CHECK-DEFAULT
+// RUN: %target-swift-frontend -emit-silgen %t/Client.swift -I %t -module-name Client -package-name mypkg -enable-library-evolution | %FileCheck %s --check-prefixes=CHECK,CHECK-DEFAULT
+
+// RUN: %target-swift-frontend -emit-silgen %t/Client.swift -I %t -module-name Client -package-name mypkg -experimental-package-bypass-resilience | %FileCheck %s --check-prefixes=CHECK,CHECK-BYPASS
+// RUN: %target-swift-frontend -emit-silgen %t/Client.swift -I %t -module-name Client -package-name mypkg -experimental-package-bypass-resilience -enable-library-evolution | %FileCheck %s --check-prefixes=CHECK,CHECK-BYPASS
+
+//--- Utils.swift
+package struct PkgStruct {
+  package var pkgVar = 1
+  package init() {}
+}
+
+public struct PubStruct {
+  public var pubVar = 1
+  public init() {}
+}
+
+//--- Client.swift
+import Utils
+
+func foo() {
+  print(PkgStruct().pkgVar)
+}
+
+// CHECK: sil hidden [ossa] @$s6Client3fooyyF : $@convention(thin) () -> () {
+// CHECK-DEFAULT: [[F_REF:%.*]] = function_ref @$s5Utils9PkgStructV6pkgVarSivg : $@convention(method) (@in_guaranteed PkgStruct) -> Int
+// CHECK-DEFAULT: sil package_external @$s5Utils9PkgStructV6pkgVarSivg : $@convention(method) (@in_guaranteed PkgStruct) -> Int
+// CHECK-BYPASS:  [[ADDR:%.*]] = struct_element_addr %7 : $*PkgStruct, #PkgStruct.pkgVar
+
+func bar() {
+  print(PubStruct().pubVar)
+}
+
+// CHECK: sil hidden [ossa] @$s6Client3baryyF : $@convention(thin) () -> () {
+// CHECK: [[F_REF:%.*]] = function_ref @$s5Utils9PubStructV6pubVarSivg : $@convention(method) (@in_guaranteed PubStruct) -> Int
+// CHECK: sil @$s5Utils9PubStructV6pubVarSivg : $@convention(method) (@in_guaranteed PubStruct) -> Int

test/Sema/package_resilience_bypass_exhaustive_switch.swift

@@ -9,7 +9,7 @@
 
 // RUN: %target-swift-frontend -typecheck %t/ClientDefault.swift -I %t -swift-version 5 -package-name mypkg -verify
 
-// RUN: %target-swift-frontend -typecheck %t/ClientOptimized.swift -I %t -swift-version 5 -package-name mypkg -package-bypass-resilience-optimization -verify
+// RUN: %target-swift-frontend -typecheck %t/ClientOptimized.swift -I %t -swift-version 5 -package-name mypkg -experimental-package-bypass-resilience -verify
 
 //--- Utils.swift