Skip to content

Commit c206725

Browse files
authored
Merge pull request #77009 from eeckstein/fix-inlining-in-mpo
MandatoryPerformanceOptimization: don't let not-inlinable functions to be inlined
2 parents b6bdafe + 709dfc2 commit c206725

File tree

8 files changed

+65
-33
lines changed

8 files changed

+65
-33
lines changed

SwiftCompilerSources/Sources/Optimizer/ModulePasses/MandatoryPerformanceOptimizations.swift

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,30 @@ private func removeUnusedMetatypeInstructions(in function: Function, _ context:
208208
}
209209

210210
private func shouldInline(apply: FullApplySite, callee: Function, alreadyInlinedFunctions: inout Set<PathFunctionTuple>) -> Bool {
211-
if callee.isTransparent {
212-
precondition(callee.hasOwnership, "transparent functions should have ownership at this stage of the pipeline")
213-
return true
211+
if let beginApply = apply as? BeginApplyInst,
212+
!beginApply.canInline
213+
{
214+
return false
215+
}
216+
217+
if !callee.canBeInlinedIntoCaller(withSerializedKind: apply.parentFunction.serializedKind) &&
218+
// Even if the serialization kind doesn't match, we need to make sure to inline witness method thunks
219+
// in embedded swift.
220+
callee.thunkKind != .thunk
221+
{
222+
return false
214223
}
215224

216-
if !apply.canInline {
225+
// Cannot inline a non-ossa function into an ossa function
226+
if apply.parentFunction.hasOwnership && !callee.hasOwnership {
217227
return false
218228
}
219229

230+
if callee.isTransparent {
231+
precondition(callee.hasOwnership, "transparent functions should have ownership at this stage of the pipeline")
232+
return true
233+
}
234+
220235
if apply is BeginApplyInst {
221236
// Avoid co-routines because they might allocate (their context).
222237
return true

SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -650,27 +650,6 @@ extension Function {
650650
}
651651

652652
extension FullApplySite {
653-
var canInline: Bool {
654-
// Some checks which are implemented in C++
655-
if !FullApplySite_canInline(bridged) {
656-
return false
657-
}
658-
// Cannot inline a non-inlinable function it an inlinable function.
659-
if let calleeFunction = referencedFunction,
660-
!calleeFunction.canBeInlinedIntoCaller(parentFunction.serializedKind) {
661-
return false
662-
}
663-
664-
// Cannot inline a non-ossa function into an ossa function
665-
if parentFunction.hasOwnership,
666-
let calleeFunction = referencedFunction,
667-
!calleeFunction.hasOwnership {
668-
return false
669-
}
670-
671-
return true
672-
}
673-
674653
var inliningCanInvalidateStackNesting: Bool {
675654
guard let calleeFunction = referencedFunction else {
676655
return false
@@ -691,6 +670,10 @@ extension FullApplySite {
691670
}
692671
}
693672

673+
extension BeginApplyInst {
674+
var canInline: Bool { BeginApply_canInline(bridged) }
675+
}
676+
694677
extension GlobalVariable {
695678
/// Removes all `begin_access` and `end_access` instructions from the initializer.
696679
///

SwiftCompilerSources/Sources/SIL/Function.swift

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,26 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
162162
}
163163
}
164164

165-
public func canBeInlinedIntoCaller(_ kind: SerializedKind) -> Bool {
166-
bridged.canBeInlinedIntoCaller(serializedKindBridged(kind))
165+
public func canBeInlinedIntoCaller(withSerializedKind callerSerializedKind: SerializedKind) -> Bool {
166+
switch serializedKind {
167+
// If both callee and caller are not_serialized, the callee can be inlined into the caller
168+
// during SIL inlining passes even if it (and the caller) might contain private symbols.
169+
case .notSerialized:
170+
return callerSerializedKind == .notSerialized;
171+
172+
// If Package-CMO is enabled, we serialize package, public, and @usableFromInline decls as
173+
// [serialized_for_package].
174+
// Their bodies must not, however, leak into @inlinable functons (that are [serialized])
175+
// since they are inlined outside of their defining module.
176+
//
177+
// If this callee is [serialized_for_package], the caller must be either non-serialized
178+
// or [serialized_for_package] for this callee's body to be inlined into the caller.
179+
// It can however be referenced by [serialized] caller.
180+
case .serializedForPackage:
181+
return callerSerializedKind != .serialized;
182+
case .serialized:
183+
return true;
184+
}
167185
}
168186

169187
public func hasValidLinkageForFragileRef(_ kind: SerializedKind) -> Bool {

include/swift/SILOptimizer/OptimizerBridging.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -401,7 +401,7 @@ struct BridgedPassContext {
401401
bool completeLifetime(BridgedValue value) const;
402402
};
403403

404-
bool FullApplySite_canInline(BridgedInstruction apply);
404+
bool BeginApply_canInline(BridgedInstruction beginApply);
405405

406406
enum class BridgedDynamicCastResult {
407407
willSucceed,

include/swift/SILOptimizer/Utils/SILInliner.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,8 @@ class SILInliner {
5858
: FuncBuilder(FuncBuilder), deleter(deleter), IKind(IKind),
5959
ApplySubs(ApplySubs) {}
6060

61+
static bool canInlineBeginApply(BeginApplyInst *BA);
62+
6163
/// Returns true if we are able to inline \arg AI.
6264
///
6365
/// *NOTE* This must be checked before attempting to inline \arg AI. If one

lib/SILOptimizer/PassManager/PassManager.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2062,9 +2062,8 @@ bool BridgedPassContext::completeLifetime(BridgedValue value) const {
20622062
return result == LifetimeCompletion::WasCompleted;
20632063
}
20642064

2065-
bool FullApplySite_canInline(BridgedInstruction apply) {
2066-
return swift::SILInliner::canInlineApplySite(
2067-
swift::FullApplySite(apply.unbridged()));
2065+
bool BeginApply_canInline(BridgedInstruction beginApply) {
2066+
return swift::SILInliner::canInlineBeginApply(beginApply.getAs<BeginApplyInst>());
20682067
}
20692068

20702069
BridgedDynamicCastResult classifyDynamicCastBridged(BridgedType sourceTy, BridgedType destTy,

lib/SILOptimizer/Utils/SILInliner.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
using namespace swift;
3131

32-
static bool canInlineBeginApply(BeginApplyInst *BA) {
32+
bool SILInliner::canInlineBeginApply(BeginApplyInst *BA) {
3333
// Don't inline if we have multiple resumption sites (i.e. end_apply or
3434
// abort_apply instructions). The current implementation clones a single
3535
// copy of the end_apply and abort_apply paths, so it can't handle values

test/embedded/dictionary-runtime.swift

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,18 +7,33 @@
77
// REQUIRES: optimized_stdlib
88
// REQUIRES: OS=macosx
99

10+
var dict1: [Int: Int] = [1:10]
11+
var dict2: [Int: Int] = [1:20]
12+
13+
func do_swap(n: Int) {
14+
swap(&dict2[n], &dict1[n])
15+
}
16+
17+
1018
@main
1119
struct Main {
1220
static func main() {
1321
var dict: [Int: StaticString] = [:]
1422
dict[11] = "hello"
1523
dict[33] = "!"
1624
dict[22] = "world"
25+
26+
// CHECK: hello world !
1727
for key in dict.keys.sorted() {
1828
print(dict[key]!, terminator: " ")
1929
}
2030
print("")
31+
32+
do_swap(n: 1)
33+
// CHECK: 20
34+
print(dict1[1]!)
35+
// CHECK: 10
36+
print(dict2[1]!)
2137
}
2238
}
2339

24-
// CHECK: hello world !

0 commit comments

Comments
 (0)