Skip to content

Commit f631dfa

Browse files
elshelsh
authored
Merge pull request #76729 from swiftlang/elsh/pcmo-ext-client
[PackageCMO] Make optimized binary module work for package-external client.
2 parents 1d3cb5c + c8d7e94 commit f631dfa

File tree

3 files changed

+162
-1
lines changed

3 files changed

+162
-1
lines changed

‎include/swift/Serialization/SerializedSILLoader.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ class SerializedSILLoader {
4747
explicit SerializedSILLoader(
4848
ASTContext &ctx, SILModule *SILMod,
4949
DeserializationNotificationHandlerSet *callbacks);
50-
5150
public:
5251
/// Create a new loader.
5352
///

‎lib/Serialization/DeserializeSIL.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -584,6 +584,25 @@ SILDeserializer::readSILFunctionChecked(DeclID FID, SILFunction *existingFn,
584584

585585
if (funcTyID == 0)
586586
return MF->diagnoseFatal("SILFunction typeID is 0");
587+
588+
// A function with [serialized_for_package] and its body should only be
589+
// deserialized in clients within the same package as its defining module;
590+
// for external clients, it should appear only as a declaration. If the
591+
// function has shared linkage, it must have been internal or private in
592+
// its defining module that's optimized, thus should remain inaccessible
593+
// outside the package boundary. This ensures that instructions, which may
594+
// only be valid in resilient mode when package optimization is enabled,
595+
// aren't inlined at the call site, preventing potential assert fails during
596+
// sil-verify.
597+
if (serializedKind == SerializedKind_t::IsSerializedForPackage) {
598+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule())) {
599+
if (rawLinkage == (unsigned int)(SILLinkage::Shared))
600+
return nullptr;
601+
if (!declarationOnly)
602+
declarationOnly = true;
603+
}
604+
}
605+
587606
auto astType = MF->getTypeChecked(funcTyID);
588607
if (!astType) {
589608
if (!existingFn || errorIfEmptyBody) {
@@ -3828,6 +3847,13 @@ SILVTable *SILDeserializer::readVTable(DeclID VId) {
38283847
return nullptr;
38293848
}
38303849

3850+
// Vtable with [serialized_for_package], i.e. optimized with Package CMO,
3851+
// should only be deserialized into a client if its defning module and the
3852+
// client are in the package.
3853+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule()) &&
3854+
SerializedKind_t(Serialized) == SerializedKind_t::IsSerializedForPackage)
3855+
return nullptr;
3856+
38313857
ClassDecl *theClass = cast<ClassDecl>(MF->getDecl(ClassID));
38323858

38333859
PrettyStackTraceDecl trace("deserializing SIL vtable for", theClass);
@@ -4201,6 +4227,13 @@ llvm::Expected<SILWitnessTable *>
42014227
MF->fatal("invalid linkage code");
42024228
}
42034229

4230+
// Witness with [serialized_for_package], i.e. optimized with Package CMO,
4231+
// should only be deserialized into a client if its defning module and the
4232+
// client are in the package.
4233+
if (!SILMod.getSwiftModule()->inSamePackage(getFile()->getParentModule()) &&
4234+
SerializedKind_t(Serialized) == SerializedKind_t::IsSerializedForPackage)
4235+
return nullptr;
4236+
42044237
// Deserialize Conformance.
42054238
auto maybeConformance = MF->getConformanceChecked(conformance);
42064239
if (!maybeConformance)

‎test/SILOptimizer/package-cmo-deserialize-for-external-client.swift

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
4+
// RUN: %target-swift-frontend -emit-module %t/Lib.swift -I %t \
5+
// RUN: -module-name Lib -package-name pkg \
6+
// RUN: -enable-library-evolution -swift-version 5 \
7+
// RUN: -O -wmo -experimental-allow-non-resilient-access -experimental-package-cmo \
8+
// RUN: -emit-module -emit-module-path %t/Lib.swiftmodule
9+
// RUN: %target-sil-opt %t/Lib.swiftmodule -o %t/Lib.sil
10+
11+
// RUN: %target-swift-frontend -emit-sil -O %t/Client.swift -I %t -package-name pkg \
12+
// RUN: -Xllvm -sil-disable-pass=DeadFunctionAndGlobalElimination -o %t/InPkgClient.sil
13+
14+
// RUN: %target-swift-frontend -emit-sil -O %t/Client.swift -I %t \
15+
// RUN: -Xllvm -sil-disable-pass=DeadFunctionAndGlobalElimination -o %t/ExtClient.sil
16+
17+
/// TEST how functions (and bodies) and tables optimized with [serialized_for_package] in Lib are
18+
/// deserialized into Client depending on package boundary.
19+
///
20+
/// Test 1: They should be deserialized into Client as Lib and Client are in the same package.
21+
// RUN: %FileCheck -check-prefix=CHECK-INPKG %s < %t/InPkgClient.sil
22+
23+
// Pub.init(_:) is removed after getting lined below.
24+
// CHECK-INPKG-NOT: @$s3Lib3PubCyACSicfc
25+
26+
// Pub.__allocating_init(_:)
27+
// CHECK-INPKG: sil public_external @$s3Lib3PubCyACSicfC : $@convention(method) (Int, @thick Pub.Type) -> @owned Pub {
28+
// Pub.init(_:) is inlined in this block, as its body was deserialized.
29+
// CHECK-INPKG: ref_element_addr {{.*}} : $Pub, #Pub.pubVar
30+
31+
// CHECK-INPKG: sil_vtable Pub {
32+
// CHECK-INPKG: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
33+
// CHECK-INPKG: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
34+
// CHECK-INPKG: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
35+
// CHECK-INPKG: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
36+
// CHECK-INPKG: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
37+
// CHECK-INPKG: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
38+
// CHECK-INPKG: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
39+
// CHECK-INPKG: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
40+
41+
// CHECK-INPKG: sil_witness_table public_external Pub: PubProto module Lib {
42+
// CHECK-INPKG: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
43+
// CHECK-INPKG: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
44+
// CHECK-INPKG: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
45+
46+
47+
/// Test 2: They should NOT be deserialized into Client as Lib and Client are NOT in the same package;
48+
/// only declaration should be deserialized, not the body.
49+
// RUN: %FileCheck -check-prefix=CHECK-EXT %s < %t/ExtClient.sil
50+
51+
// CHECK-EXT-NOT: sil_vtable Pub {
52+
// CHECK-EXT-NOT: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
53+
// CHECK-EXT-NOT: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
54+
// CHECK-EXT-NOT: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
55+
// CHECK-EXT-NOT: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
56+
// CHECK-EXT-NOT: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
57+
// CHECK-EXT-NOT: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
58+
// CHECK-EXT-NOT: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
59+
// CHECK-EXT-NOT: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
60+
61+
// CHECK-EXT-NOT: sil_witness_table public_external Pub: PubProto module Lib {
62+
// CHECK-EXT-NOT: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
63+
// CHECK-EXT-NOT: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
64+
// CHECK-EXT-NOT: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
65+
66+
// Pub.init(_:) is just a decl; does not contain the body.
67+
// CHECK-EXT: sil @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub
68+
69+
70+
/// Verify functions and tables are optimized with Package CMO in Lib.
71+
// RUN: %FileCheck -check-prefix=CHECK-LIB %s < %t/Lib.sil
72+
73+
// CHECK-LIB: sil [serialized] [exact_self_class] [canonical] @$s3Lib3PubCyACSicfC : $@convention(method) (Int, @thick Pub.Type) -> @owned Pub {
74+
// CHECK-LIB: function_ref @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub
75+
// Pub.init(_:)
76+
77+
// CHECK-LIB: sil [serialized_for_package] [canonical] @$s3Lib3PubCyACSicfc : $@convention(method) (Int, @owned Pub) -> @owned Pub {
78+
// CHECK-LIB: ref_element_addr {{.*}} : $Pub, #Pub.pubVar
79+
// Pub.__allocating_init(_:)
80+
81+
// Pub.pkgVar.getter
82+
// CHECK-LIB: sil package [serialized_for_package] [canonical] @$s3Lib3PubC6pkgVarSivg : $@convention(method) (@guaranteed Pub) -> Int {
83+
84+
// CHECK-LIB: sil_vtable [serialized_for_package] Pub {
85+
// CHECK-LIB: #Pub.pubVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pubVarSivg // Pub.pubVar.getter
86+
// CHECK-LIB: #Pub.pubVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pubVarSivs // Pub.pubVar.setter
87+
// CHECK-LIB: #Pub.pubVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pubVarSivM // Pub.pubVar.modify
88+
// CHECK-LIB: #Pub.pkgVar!getter: (Pub) -> () -> Int : @$s3Lib3PubC6pkgVarSivg // Pub.pkgVar.getter
89+
// CHECK-LIB: #Pub.pkgVar!setter: (Pub) -> (Int) -> () : @$s3Lib3PubC6pkgVarSivs // Pub.pkgVar.setter
90+
// CHECK-LIB: #Pub.pkgVar!modify: (Pub) -> () -> () : @$s3Lib3PubC6pkgVarSivM // Pub.pkgVar.modify
91+
// CHECK-LIB: #Pub.init!allocator: (Pub.Type) -> (Int) -> Pub : @$s3Lib3PubCyACSicfC // Pub.__allocating_init(_:)
92+
// CHECK-LIB: #Pub.deinit!deallocator: @$s3Lib3PubCfD // Pub.__deallocating_deinit
93+
94+
// CHECK-LIB: sil_witness_table [serialized_for_package] Pub: PubProto module Lib {
95+
// CHECK-LIB: method #PubProto.pubVar!getter: <Self where Self : PubProto> (Self) -> () -> Int : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivgTW // protocol witness for PubProto.pubVar.getter in conformance Pub
96+
// CHECK-LIB: method #PubProto.pubVar!setter: <Self where Self : PubProto> (inout Self) -> (Int) -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivsTW // protocol witness for PubProto.pubVar.setter in conformance Pub
97+
// CHECK-LIB: method #PubProto.pubVar!modify: <Self where Self : PubProto> (inout Self) -> () -> () : @$s3Lib3PubCAA0B5ProtoA2aDP6pubVarSivMTW // protocol witness for PubProto.pubVar.modify in conformance Pub
98+
99+
100+
//--- Lib.swift
101+
102+
public protocol PubProto {
103+
var pubVar: Int { get set }
104+
}
105+
106+
public class Pub: PubProto {
107+
public var pubVar: Int = 1
108+
package var pkgVar: Int = 2
109+
public init(_ arg: Int) {
110+
pubVar = arg
111+
pkgVar = arg
112+
}
113+
}
114+
115+
//--- Client.swift
116+
import Lib
117+
118+
public func test(_ arg: Int) -> Int {
119+
let x = Pub(arg)
120+
x.pubVar = 3
121+
return x.run()
122+
}
123+
124+
public extension PubProto {
125+
func run() -> Int {
126+
return pubVar
127+
}
128+
}
129+

0 commit comments

Comments
 (0)