Skip to content

Commit 67594f1

Browse files
committed
Package CMO: Enable serializing decls imported with @_spiOnly or package import.
Starting in Swift 6.0, `package` access level and `@_spiOnly` attribute have been increasingly used in import statements. However, existing import filtering prevented serialization of package APIs that included such decls, leading to a significant drop in overall serialization. This PR removes these restrictive filters, and allows decls from SDK or system modules to be included in serialization. rdar://130788606
1 parent 4ae5685 commit 67594f1

File tree

4 files changed

+119
-34
lines changed

4 files changed

+119
-34
lines changed

include/swift/AST/Module.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,11 +1046,6 @@ class ModuleDecl
10461046
/// This assumes that \p module was imported.
10471047
bool isImportedImplementationOnly(const ModuleDecl *module) const;
10481048

1049-
/// Returns true if decl context or its content can be serialized by
1050-
/// cross-module-optimization.
1051-
/// The \p ctxt can e.g. be a NominalType or the context of a function.
1052-
bool canBeUsedForCrossModuleOptimization(DeclContext *ctxt) const;
1053-
10541049
/// Finds all top-level decls of this module.
10551050
///
10561051
/// This does a simple local lookup, not recursively looking through imports.

lib/AST/Module.cpp

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3047,33 +3047,6 @@ bool ModuleDecl::isImportedImplementationOnly(const ModuleDecl *module) const {
30473047
return true;
30483048
}
30493049

3050-
bool ModuleDecl::
3051-
canBeUsedForCrossModuleOptimization(DeclContext *ctxt) const {
3052-
ModuleDecl *moduleOfCtxt = ctxt->getParentModule();
3053-
3054-
// If the context defined in the same module - or is the same module, it's
3055-
// fine.
3056-
if (moduleOfCtxt == this)
3057-
return true;
3058-
3059-
// See if context is imported in a "regular" way, i.e. not with
3060-
// @_implementationOnly, `package import` or @_spiOnly.
3061-
ModuleDecl::ImportFilter filter = {
3062-
ModuleDecl::ImportFilterKind::ImplementationOnly,
3063-
ModuleDecl::ImportFilterKind::PackageOnly,
3064-
ModuleDecl::ImportFilterKind::SPIOnly
3065-
};
3066-
SmallVector<ImportedModule, 4> results;
3067-
getImportedModules(results, filter);
3068-
3069-
auto &imports = getASTContext().getImportCache();
3070-
for (auto &desc : results) {
3071-
if (imports.isImportedBy(moduleOfCtxt, desc.importedModule))
3072-
return false;
3073-
}
3074-
return true;
3075-
}
3076-
30773050
void SourceFile::lookupImportedSPIGroups(
30783051
const ModuleDecl *importedModule,
30793052
llvm::SmallSetVector<Identifier, 4> &spiGroups) const {

lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#define DEBUG_TYPE "cross-module-serialization-setup"
1818
#include "swift/AST/Module.h"
19+
#include "swift/AST/ImportCache.h"
1920
#include "swift/Basic/Assertions.h"
2021
#include "swift/IRGen/TBDGen.h"
2122
#include "swift/SIL/ApplySite.h"
@@ -103,6 +104,11 @@ class CrossModuleOptimization {
103104
bool canSerializeType(CanType type);
104105
bool canSerializeDecl(NominalTypeDecl *decl);
105106

107+
/// Check whether decls imported with certain access levels or attributes
108+
/// can be serialized.
109+
/// The \p ctxt can e.g. be a NominalType or the context of a function.
110+
bool checkImports(DeclContext *ctxt) const;
111+
106112
bool canUseFromInline(DeclContext *declCtxt);
107113

108114
bool canUseFromInline(SILFunction *func);
@@ -734,7 +740,12 @@ static bool couldBeLinkedStatically(DeclContext *funcCtxt, SILModule &module) {
734740
// The stdlib module is always linked dynamically.
735741
if (funcModule == module.getASTContext().getStdlibModule())
736742
return false;
737-
743+
744+
// An sdk or system module should be linked dynamically.
745+
if (isPackageCMOEnabled(module.getSwiftModule()) &&
746+
funcModule->isNonUserModule())
747+
return false;
748+
738749
// Conservatively assume the function is in a statically linked module.
739750
return true;
740751
}
@@ -744,7 +755,7 @@ bool CrossModuleOptimization::canUseFromInline(DeclContext *declCtxt) {
744755
if (everything)
745756
return true;
746757

747-
if (!M.getSwiftModule()->canBeUsedForCrossModuleOptimization(declCtxt))
758+
if (!checkImports(declCtxt))
748759
return false;
749760

750761
/// If we are emitting a TBD file, the TBD file only contains public symbols
@@ -760,6 +771,52 @@ bool CrossModuleOptimization::canUseFromInline(DeclContext *declCtxt) {
760771
return true;
761772
}
762773

774+
bool CrossModuleOptimization::checkImports(DeclContext *ctxt) const {
775+
ModuleDecl *moduleOfCtxt = ctxt->getParentModule();
776+
777+
// If the context defined in the same module - or is the same module, it's
778+
// fine.
779+
if (moduleOfCtxt == M.getSwiftModule())
780+
return true;
781+
782+
ModuleDecl::ImportFilter filter;
783+
784+
if (isPackageCMOEnabled(M.getSwiftModule())) {
785+
// If Package CMO is enabled, decls imported with `package import`
786+
// or `@_spiOnly import` into this module should be allowed to be
787+
// serialized. They are used in decls with `package` or higher
788+
// access level, with or without @_spi; a client of this module
789+
// should be able to access them directly if in the same package.
790+
filter = { ModuleDecl::ImportFilterKind::ImplementationOnly };
791+
} else {
792+
// See if context is imported in a "regular" way, i.e. not with
793+
// @_implementationOnly, `package import` or @_spiOnly.
794+
filter = {
795+
ModuleDecl::ImportFilterKind::ImplementationOnly,
796+
ModuleDecl::ImportFilterKind::PackageOnly,
797+
ModuleDecl::ImportFilterKind::SPIOnly
798+
};
799+
}
800+
SmallVector<ImportedModule, 4> results;
801+
M.getSwiftModule()->getImportedModules(results, filter);
802+
803+
auto &imports = M.getSwiftModule()->getASTContext().getImportCache();
804+
for (auto &desc : results) {
805+
if (imports.isImportedBy(moduleOfCtxt, desc.importedModule)) {
806+
// E.g. `@_implementationOnly import QuartzCore_Private.CALayerPrivate`
807+
// imports `Foundation` as its transitive dependency module; use of a
808+
// a `public` decl in `Foundation` such as `IndexSet` in a function
809+
// signature should not block serialization in Package CMO given the
810+
// function has `package` or higher access level.
811+
if (isPackageCMOEnabled(M.getSwiftModule()) &&
812+
moduleOfCtxt->isNonUserModule())
813+
continue;
814+
return false;
815+
}
816+
}
817+
return true;
818+
}
819+
763820
/// Returns true if the function \p func can be used from a serialized function.
764821
bool CrossModuleOptimization::canUseFromInline(SILFunction *function) {
765822
if (everything)
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// RUN: %target-swift-frontend %t/CoreA.swift \
5+
// RUN: -module-name=CoreA -package-name Pkg \
6+
// RUN: -parse-as-library -emit-module \
7+
// RUN: -emit-module-path %t/CoreA.swiftmodule -I%t \
8+
// RUN: -O -wmo -enable-library-evolution
9+
10+
// RUN: %target-swift-frontend %t/CoreB.swift \
11+
// RUN: -module-name=CoreB -package-name Pkg \
12+
// RUN: -parse-as-library -emit-module \
13+
// RUN: -emit-module-path %t/CoreB.swiftmodule -I%t \
14+
// RUN: -O -wmo -enable-library-evolution
15+
16+
// RUN: %target-swift-frontend %t/Lib.swift \
17+
// RUN: -module-name=Lib -package-name Pkg \
18+
// RUN: -parse-as-library -emit-module \
19+
// RUN: -experimental-spi-only-imports \
20+
// RUN: -emit-module-path %t/Lib.swiftmodule -I %t \
21+
// RUN: -experimental-package-cmo -experimental-allow-non-resilient-access \
22+
// RUN: -O -wmo -enable-library-evolution -Rmodule-loading 2> %t/Lib-result.txt
23+
// RUN: %target-sil-opt %t/Lib.swiftmodule -I %t -sil-verify-all -o %t/Lib.sil
24+
// RUN: %FileCheck %s < %t/Lib.sil
25+
26+
// REQUIRES: swift_in_compiler
27+
28+
29+
//--- Lib.swift
30+
package import CoreA
31+
@_spiOnly public import CoreB
32+
33+
/// PkgStruct is imported with `package import` and should be serialized.
34+
// CHECK-DAG: sil package [serialized_for_package] [canonical] @$s3Lib7libFuncyy5CoreA9PkgStructVF : $@convention(thin) (@in_guaranteed PkgStruct) -> () {
35+
package func libFunc(_ arg: PkgStruct) {
36+
print(arg.pkgVar)
37+
}
38+
39+
/// PubStruct is imported with `@_spiOnly public import` and should be serialized.
40+
// CHECK-DAG: sil [serialized_for_package] [canonical] @$s3Lib7spiFuncyy5CoreB15PubStructForSPIVF : $@convention(thin) (@in_guaranteed PubStructForSPI) -> () {
41+
@_spi(InCoreB)
42+
public func spiFunc(_ arg: PubStructForSPI) {
43+
print(arg.pubVarForSPI)
44+
}
45+
46+
//--- CoreA.swift
47+
package struct PkgStruct {
48+
package var pkgVar: Int
49+
package init(_ arg: Int) {
50+
self.pkgVar = arg
51+
}
52+
}
53+
54+
//--- CoreB.swift
55+
public struct PubStructForSPI {
56+
public var pubVarForSPI: String
57+
public init(_ arg: String) {
58+
self.pubVarForSPI = arg
59+
}
60+
}

0 commit comments

Comments
 (0)