Skip to content

Commit e3f2bb7

Browse files
committed
[inliner] Add a new Inliner that only inlines AlwaysInline functions (but do not put it in the pass pipeline).
We need this anyways for -Onone and I want to do some experiments with running this very early so I can expose more of the stdlib (modulo inlining) to the new ownership optimizing passes. I also changed how the inliner handles inlining around OSSA by changing it to check early that if the caller is in ossa, then we only inline if all of the callees that the caller calls are in ossa. The intention is to hopefully avoid weird swings in code-size/perf due to the inliner heuristic's calculation being artificially manipulated due to some callees not being available to inline (due to this difference) when others are already available.
1 parent 11df7a8 commit e3f2bb7

File tree

5 files changed

+82
-12
lines changed

5 files changed

+82
-12
lines changed

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,8 @@ PASS(NonTransparentFunctionOwnershipModelEliminator,
243243
"Eliminate Ownership Annotations from non-transparent SIL Functions")
244244
PASS(RCIdentityDumper, "rc-id-dumper",
245245
"Print Reference Count Identities")
246+
PASS(AlwaysInlineInliner, "always-inline",
247+
"Inline always inline functions")
246248
PASS(PerfInliner, "inline",
247249
"Performance Function Inlining")
248250
PASS(PerformanceConstantPropagation, "performance-constant-propagation",

include/swift/SILOptimizer/Utils/PerformanceInlinerUtils.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class SideEffectAnalysis;
3535
enum class InlineSelection {
3636
Everything,
3737
NoGlobalInit, // and no availability semantics calls
38-
NoSemanticsAndGlobalInit
38+
NoSemanticsAndGlobalInit,
39+
OnlyInlineAlways,
3940
};
4041

4142
// Returns the callee of an apply_inst if it is basically inlinable.

lib/SILOptimizer/Transforms/PerformanceInliner.cpp

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,11 @@ llvm::cl::opt<bool>
4444
EnableSILAggressiveInlining("sil-aggressive-inline", llvm::cl::init(false),
4545
llvm::cl::desc("Enable aggressive inlining"));
4646

47+
llvm::cl::opt<bool> EnableVerifyAfterInlining(
48+
"sil-inline-verify-after-inline", llvm::cl::init(false),
49+
llvm::cl::desc("Run sil verification after inlining all found callee apply "
50+
"sites into a caller."));
51+
4752
//===----------------------------------------------------------------------===//
4853
// Performance Inliner
4954
//===----------------------------------------------------------------------===//
@@ -919,11 +924,9 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
919924
}
920925

921926
// If we have a callee that doesn't have ownership, but the caller does have
922-
// ownership... do not inline. The two modes are incompatible. Today this
923-
// should only happen with transparent functions.
927+
// ownership... do not inline. The two modes are incompatible, so skip this
928+
// apply site for now.
924929
if (!Callee->hasOwnership() && Caller->hasOwnership()) {
925-
assert(Caller->isTransparent() &&
926-
"Should only happen with transparent functions");
927930
continue;
928931
}
929932

@@ -953,6 +956,13 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
953956
StackNesting().correctStackNesting(Caller);
954957
}
955958

959+
// If we were asked to verify our caller after inlining all callees we could
960+
// find into it, do so now. This makes it easier to catch verification bugs in
961+
// the inliner without running the entire inliner.
962+
if (EnableVerifyAfterInlining) {
963+
Caller->verify();
964+
}
965+
956966
return true;
957967
}
958968

@@ -1027,6 +1037,11 @@ class SILPerformanceInlinerPass : public SILFunctionTransform {
10271037
};
10281038
} // end anonymous namespace
10291039

1040+
SILTransform *swift::createAlwaysInlineInliner() {
1041+
return new SILPerformanceInlinerPass(InlineSelection::OnlyInlineAlways,
1042+
"InlineAlways");
1043+
}
1044+
10301045
/// Create an inliner pass that does not inline functions that are marked with
10311046
/// the @_semantics, @_effects or global_init attributes.
10321047
SILTransform *swift::createEarlyInliner() {

lib/SILOptimizer/Utils/PerformanceInlinerUtils.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -637,14 +637,21 @@ SILFunction *swift::getEligibleFunction(FullApplySite AI,
637637
if (!SILInliner::canInlineApplySite(AI))
638638
return nullptr;
639639

640+
// If our inline selection is only always inline, do a quick check if we have
641+
// an always inline function and bail otherwise.
642+
if (WhatToInline == InlineSelection::OnlyInlineAlways &&
643+
Callee->getInlineStrategy() != AlwaysInline) {
644+
return nullptr;
645+
}
646+
640647
ModuleDecl *SwiftModule = Callee->getModule().getSwiftModule();
641648
bool IsInStdlib = (SwiftModule->isStdlibModule() ||
642649
SwiftModule->isOnoneSupportModule());
643650

644651
// Don't inline functions that are marked with the @_semantics or @_effects
645652
// attribute if the inliner is asked not to inline them.
646653
if (Callee->hasSemanticsAttrs() || Callee->hasEffectsKind()) {
647-
if (WhatToInline == InlineSelection::NoSemanticsAndGlobalInit) {
654+
if (WhatToInline >= InlineSelection::NoSemanticsAndGlobalInit) {
648655
if (shouldSkipApplyDuringEarlyInlining(AI))
649656
return nullptr;
650657
if (Callee->hasSemanticsAttr("inline_late"))
@@ -692,13 +699,14 @@ SILFunction *swift::getEligibleFunction(FullApplySite AI,
692699
// Check if passed Self is the same as the Self of the caller.
693700
// In this case, it is safe to inline because both functions
694701
// use the same Self.
695-
if (AI.hasSelfArgument() && Caller->hasSelfMetadataParam()) {
696-
auto CalleeSelf = stripCasts(AI.getSelfArgument());
697-
auto CallerSelf = Caller->getSelfMetadataArgument();
698-
if (CalleeSelf != SILValue(CallerSelf))
699-
return nullptr;
700-
} else
702+
if (!AI.hasSelfArgument() || !Caller->hasSelfMetadataParam()) {
701703
return nullptr;
704+
}
705+
auto CalleeSelf = stripCasts(AI.getSelfArgument());
706+
auto CallerSelf = Caller->getSelfMetadataArgument();
707+
if (CalleeSelf != SILValue(CallerSelf)) {
708+
return nullptr;
709+
}
702710
}
703711

704712
// Detect self-recursive calls.
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// RUN: %target-sil-opt -enable-sil-verify-all %s -always-inline | %FileCheck %s
2+
3+
sil @doSomething1 : $@convention(thin) () -> ()
4+
sil @doSomething2 : $@convention(thin) () -> ()
5+
sil @doSomething3 : $@convention(thin) () -> ()
6+
7+
sil [ossa] [always_inline] @do_inline_this : $@convention(thin) () -> () {
8+
bb0:
9+
%d1 = function_ref @doSomething1 : $@convention(thin) () -> ()
10+
apply %d1() : $@convention(thin) () -> ()
11+
%9999 = tuple()
12+
return %9999 : $()
13+
}
14+
15+
sil [ossa] @donot_inline_this : $@convention(thin) () -> () {
16+
bb0:
17+
%d1 = function_ref @doSomething2 : $@convention(thin) () -> ()
18+
apply %d1() : $@convention(thin) () -> ()
19+
%9999 = tuple()
20+
return %9999 : $()
21+
}
22+
23+
sil [ossa] @empty_function : $@convention(thin) () -> () {
24+
bb0:
25+
%9999 = tuple()
26+
return %9999 : $()
27+
}
28+
29+
// CHECK-LABEL: sil [ossa] @caller : $@convention(thin) () -> () {
30+
// CHECK-NOT: function_ref @do_inline_this : $@convention(thin) () -> ()
31+
// CHECK: function_ref @donot_inline_this : $@convention(thin) () -> ()
32+
// CHECK: function_ref @empty_function : $@convention(thin) () -> ()
33+
// CHECK: } // end sil function 'caller'
34+
sil [ossa] @caller : $@convention(thin) () -> () {
35+
bb0:
36+
%c1 = function_ref @do_inline_this : $@convention(thin) () -> ()
37+
apply %c1() : $@convention(thin) () -> ()
38+
%c2 = function_ref @donot_inline_this : $@convention(thin) () -> ()
39+
apply %c2() : $@convention(thin) () -> ()
40+
%c3 = function_ref @empty_function : $@convention(thin) () -> ()
41+
apply %c3() : $@convention(thin) () -> ()
42+
%9999 = tuple()
43+
return %9999 : $()
44+
}

0 commit comments

Comments
 (0)