Skip to content

Commit 2e4a010

Browse files
eecksteineeckstein
authored
Merge pull request #40721 from eeckstein/cmo-by-default
Do conservative cross-module-optimization by default
2 parents 02c7eba + 6ee19f0 commit 2e4a010

11 files changed

+69
-25
lines changed

‎lib/SILOptimizer/IPO/CrossModuleOptimization.cpp

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,16 @@ class CrossModuleOptimization {
5555

5656
SILModule &M;
5757

58+
/// True, if CMO runs by default.
59+
/// In this case, serialization decisions are made very conservatively to
60+
/// avoid code size increase.
61+
bool conservative;
62+
5863
typedef llvm::DenseMap<SILFunction *, bool> FunctionFlags;
5964

6065
public:
61-
CrossModuleOptimization(SILModule &M)
62-
: M(M) { }
66+
CrossModuleOptimization(SILModule &M, bool conservative)
67+
: M(M), conservative(conservative) { }
6368

6469
void serializeFunctionsInModule();
6570

@@ -255,6 +260,12 @@ bool CrossModuleOptimization::canSerializeInstruction(SILInstruction *inst,
255260
if (!canUseFromInline(callee))
256261
return false;
257262

263+
// In conservative mode we don't want to turn non-public functions into
264+
// public functions, because that can increase code size. E.g. if the
265+
// function is completely inlined afterwards.
266+
if (conservative && callee->getLinkage() != SILLinkage::Public)
267+
return false;
268+
258269
return true;
259270
}
260271
if (auto *KPI = dyn_cast<KeyPathInst>(inst)) {
@@ -273,6 +284,13 @@ bool CrossModuleOptimization::canSerializeInstruction(SILInstruction *inst,
273284
if (auto *MI = dyn_cast<MethodInst>(inst)) {
274285
return !MI->getMember().isForeign;
275286
}
287+
if (auto *REAI = dyn_cast<RefElementAddrInst>(inst)) {
288+
// In conservative mode, we don't support class field accesse of non-public
289+
// properties, because that would require to make the field decl public -
290+
// which keeps more metadata alive.
291+
return !conservative ||
292+
REAI->getField()->getEffectiveAccess() >= AccessLevel::Public;
293+
}
276294
return true;
277295
}
278296

@@ -298,6 +316,10 @@ bool CrossModuleOptimization::canSerializeType(SILType type) {
298316
CanType subType = rawSubType->getCanonicalType();
299317
if (NominalTypeDecl *subNT = subType->getNominalOrBoundGenericNominal()) {
300318

319+
if (conservative && subNT->getEffectiveAccess() < AccessLevel::Public) {
320+
return true;
321+
}
322+
301323
// Exclude types which are defined in an @_implementationOnly imported
302324
// module. Such modules are not transitively available.
303325
if (!M.getSwiftModule()->canBeUsedForCrossModuleOptimization(subNT)) {
@@ -348,13 +370,15 @@ bool CrossModuleOptimization::shouldSerialize(SILFunction *function) {
348370
if (SerializeEverything)
349371
return true;
350372

351-
// The basic heursitic: serialize all generic functions, because it makes a
352-
// huge difference if generic functions can be specialized or not.
353-
if (function->getLoweredFunctionType()->isPolymorphic())
354-
return true;
373+
if (!conservative) {
374+
// The basic heursitic: serialize all generic functions, because it makes a
375+
// huge difference if generic functions can be specialized or not.
376+
if (function->getLoweredFunctionType()->isPolymorphic())
377+
return true;
355378

356-
if (function->getLinkage() == SILLinkage::Shared)
357-
return true;
379+
if (function->getLinkage() == SILLinkage::Shared)
380+
return true;
381+
}
358382

359383
// Also serialize "small" non-generic functions.
360384
int size = 0;
@@ -401,8 +425,27 @@ void CrossModuleOptimization::serializeInstruction(SILInstruction *inst,
401425
if (!callee->isDefinition() || callee->isAvailableExternally())
402426
return;
403427
if (canUseFromInline(callee)) {
404-
// Make the function 'public'.
405-
makeFunctionUsableFromInline(callee);
428+
if (conservative) {
429+
// In conservative mode, avoid making non-public functions public,
430+
// because that can increase code size.
431+
if (callee->getLinkage() == SILLinkage::Private ||
432+
callee->getLinkage() == SILLinkage::Hidden) {
433+
if (callee->getEffectiveSymbolLinkage() == SILLinkage::Public) {
434+
// It's a internal/private class method. There is no harm in making
435+
// it public, because it gets public symbol linkage anyway.
436+
makeFunctionUsableFromInline(callee);
437+
} else {
438+
// Treat the function like a 'shared' function, e.g. like a
439+
// specialization. This is better for code size than to make it
440+
// public, because in conservative mode we are only do this for very
441+
// small functions.
442+
callee->setLinkage(SILLinkage::Shared);
443+
}
444+
}
445+
} else {
446+
// Make the function 'public'.
447+
makeFunctionUsableFromInline(callee);
448+
}
406449
}
407450
serializeFunction(callee, canSerializeFlags);
408451
assert(callee->isSerialized() == IsSerialized ||
@@ -531,10 +574,9 @@ class CrossModuleOptimizationPass: public SILModuleTransform {
531574
return;
532575
if (!M.isWholeModule())
533576
return;
534-
if (!M.getOptions().CrossModuleOptimization)
535-
return;
536577

537-
CrossModuleOptimization CMO(M);
578+
CrossModuleOptimization CMO(M,
579+
/*conservative*/ !M.getOptions().CrossModuleOptimization);
538580
CMO.serializeFunctionsInModule();
539581
}
540582
};

‎test/AutoDiff/TBD/derivative_symbols.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %target-swift-frontend -emit-ir -o/dev/null -parse-as-library -module-name test -validate-tbd-against-ir=all %s
2-
// RUN: %target-swift-frontend -emit-ir -o/dev/null -parse-as-library -module-name test -validate-tbd-against-ir=all %s -O
1+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -Xllvm -sil-disable-pass=cmo -parse-as-library -module-name test -validate-tbd-against-ir=all %s
2+
// RUN: %target-swift-frontend -emit-ir -o/dev/null -Xllvm -sil-disable-pass=cmo -parse-as-library -module-name test -validate-tbd-against-ir=all %s -O
33
// RUN: %target-swift-frontend -emit-ir -o/dev/null -parse-as-library -module-name test -validate-tbd-against-ir=missing %s -enable-testing
44
// RUN: %target-swift-frontend -emit-ir -o/dev/null -parse-as-library -module-name test -validate-tbd-against-ir=missing %s -enable-testing -O
55

‎test/Interpreter/SDK/mixed_mode_class_with_missing_properties.swift

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// RUN: %empty-directory(%t)
22
// RUN: cp %s %t/main.swift
3-
// RUN: %target-build-swift -whole-module-optimization -emit-module-path %t/UsingObjCStuff.swiftmodule -c -o %t/UsingObjCStuff.o -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 5 -parse-as-library %S/Inputs/mixed_mode/UsingObjCStuff.swift
3+
4+
// CMO does not work because of a deserialization failure: rdar://86575886
5+
// RUN: %target-build-swift -Xllvm -sil-disable-pass=cmo -whole-module-optimization -emit-module-path %t/UsingObjCStuff.swiftmodule -c -o %t/UsingObjCStuff.o -module-name UsingObjCStuff -I %t -I %S/Inputs/mixed_mode -swift-version 5 -parse-as-library %S/Inputs/mixed_mode/UsingObjCStuff.swift
46
// RUN: %target-build-swift -o %t/a.out.v4 -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 4 %t/main.swift %t/UsingObjCStuff.o
57
// RUN: %target-build-swift -o %t/a.out.v5 -I %t -I %S/Inputs/mixed_mode -module-name main -swift-version 5 %t/main.swift %t/UsingObjCStuff.o
68
// RUN: %target-codesign %t/a.out.v4

‎test/SIL/Parser/array_roundtrip.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-swift-frontend %s -emit-sil -Ounchecked | %target-sil-opt
1+
// RUN: %target-swift-frontend %s -emit-sil -Ounchecked -Xllvm -sil-disable-pass=cmo | %target-sil-opt
22

33
// Fails if the positions of the two Collection subscript requirements are
44
// reversed. rdar://problem/46650834

‎test/SILGen/fragile_globals.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ var mygg = 29
1111
// Check if we have one token: from mygg.
1212
// Initializers from other modules are never fragile.
1313

14-
// CHECK: sil_global private{{.*}} @[[T3:.*]]Wz
14+
// CHECK: sil_global {{.*}} @[[T3:.*]]Wz
1515

1616
//@inlinable
1717
public func sum() -> Int {

‎test/SILOptimizer/access_enforcement_options.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public func accessIntTwice() {
2828
// CHECK-LABEL: } // end sil function '$s26access_enforcement_options0A8IntTwiceyyF'
2929

3030
// closure #1 in accessIntTwice()
31-
// CHECK-LABEL: sil private @$s26access_enforcement_options0A8IntTwiceyyFyycfU_ : $@convention(thin) (@guaranteed { var Int }) -> () {
31+
// CHECK-LABEL: sil {{.*}}@$s26access_enforcement_options0A8IntTwiceyyFyycfU_ : $@convention(thin) (@guaranteed { var Int }) -> () {
3232
// CHECK: bb0(%0 : ${ var Int }):
3333
// CHECK: [[PROJ:%.*]] = project_box %0 : ${ var Int }, 0
3434
// NONE: [[ACCESS:%.*]] = begin_access [modify] [dynamic] [[PROJ]] : $*Int

‎test/SILOptimizer/dead_function_elimination.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ public func keepPtrAlive() {
198198
// CHECK-TESTING: sil {{.*}}publicClassMethod
199199
// CHECK-TESTING: sil {{.*}}DeadWitness
200200

201-
// CHECK-LABEL: sil_global hidden @$s25dead_function_elimination5GFStrV12aliveFuncPtryycvpZ
201+
// CHECK-LABEL: sil_global {{.*}}@$s25dead_function_elimination5GFStrV12aliveFuncPtryycvpZ
202202
// CHECK-LABEL: @$s25dead_function_elimination14donotEliminateyyF
203203
// CHECK-LABEL: sil @$s25dead_function_elimination12keepPtrAliveyyF
204204

‎test/SILOptimizer/dont_remove_dynamic_self_arg.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ func transform<T>(_ a: Any, c: (Any) -> T?) -> T? {
99
return c(a)
1010
}
1111

12-
// CHECK-LABEL: sil private @$s4test1XC6testityACSgypFACXDSgypXEfU_ :
12+
// CHECK-LABEL: sil {{.*}}@$s4test1XC6testityACSgypFACXDSgypXEfU_ :
1313
// CHECK: bb0({{.*}}, {{%[0-9]+}} : $@thick @dynamic_self X.Type):
1414

1515
// The checked_cast_addr_br must have %1 as implicit type dependend operand.

‎test/Serialization/xref-private-type.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
2-
// RUN: %target-build-swift -swift-version 4 -O -whole-module-optimization -emit-module-path %t/LibCore.swiftmodule %S/Inputs/xref-private-type/LibCore.swift
3-
// RUN: %target-build-swift -swift-version 4 -O -I %t -whole-module-optimization -emit-module-path %t/Lib.swiftmodule %S/Inputs/xref-private-type/Lib.swift
2+
// RUN: %target-build-swift -swift-version 4 -O -Xllvm -sil-disable-pass=cmo -whole-module-optimization -emit-module-path %t/LibCore.swiftmodule %S/Inputs/xref-private-type/LibCore.swift
3+
// RUN: %target-build-swift -swift-version 4 -O -Xllvm -sil-disable-pass=cmo -I %t -whole-module-optimization -emit-module-path %t/Lib.swiftmodule %S/Inputs/xref-private-type/Lib.swift
44
// RUN: %target-build-swift -swift-version 4 -O -I %t -emit-sil %s | %FileCheck %s
55

66
import Lib

‎test/TBD/all-in-one.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// RUN: %target-build-swift -module-name all_in_one -emit-module-path %t/all_in_one.swiftmodule -emit-tbd-path %t/wmo_Onone.tbd %S/class.swift %t/class_objc.swift %S/enum.swift %t/extension.swift %S/function.swift %S/global.swift %S/main.swift %S/protocol.swift %S/struct.swift %t/subclass.swift -I %t -import-objc-header %S/Inputs/objc_class_header.h -Xfrontend -disable-objc-attr-requires-foundation-module -wmo
1313

1414
// RUN: %target-build-swift -module-name all_in_one -emit-module-path %t/all_in_one.swiftmodule -emit-tbd-path %t/incremental_O.tbd %S/class.swift %t/class_objc.swift %S/enum.swift %t/extension.swift %S/function.swift %S/global.swift %S/main.swift %S/protocol.swift %S/struct.swift %t/subclass.swift -I %t -import-objc-header %S/Inputs/objc_class_header.h -Xfrontend -disable-objc-attr-requires-foundation-module -O
15-
// RUN: %target-build-swift -module-name all_in_one -emit-module-path %t/all_in_one.swiftmodule -emit-tbd-path %t/wmo_O.tbd %S/class.swift %t/class_objc.swift %S/enum.swift %t/extension.swift %S/function.swift %S/global.swift %S/main.swift %S/protocol.swift %S/struct.swift %t/subclass.swift -I %t -import-objc-header %S/Inputs/objc_class_header.h -Xfrontend -disable-objc-attr-requires-foundation-module -wmo -O
15+
// RUN: %target-build-swift -module-name all_in_one -emit-module-path %t/all_in_one.swiftmodule -emit-tbd-path %t/wmo_O.tbd %S/class.swift %t/class_objc.swift %S/enum.swift %t/extension.swift %S/function.swift %S/global.swift %S/main.swift %S/protocol.swift %S/struct.swift %t/subclass.swift -I %t -import-objc-header %S/Inputs/objc_class_header.h -Xfrontend -disable-objc-attr-requires-foundation-module -wmo -O -Xllvm -sil-disable-pass=cmo
1616

1717
// RUN: diff %t/incremental_Onone.tbd %t/wmo_Onone.tbd
1818
// RUN: diff %t/incremental_O.tbd %t/wmo_O.tbd

‎test/TBD/specialization.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// REQUIRES: VENDOR=apple
22
// Validate the the specializations actually exist (if they don't then we're not
33
// validating that they end up with the correct linkages):
4-
// RUN: %target-swift-frontend -emit-sil -o- -O -validate-tbd-against-ir=none %s | %FileCheck %s
4+
// RUN: %target-swift-frontend -emit-sil -o- -O -Xllvm -sil-disable-pass=cmo -validate-tbd-against-ir=none %s | %FileCheck %s
55

66
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all %s
77
// RUN: %target-swift-frontend -emit-ir -o/dev/null -O -validate-tbd-against-ir=all -enable-library-evolution %s

0 commit comments

Comments
 (0)