Skip to content

Commit 83bc199

Browse files
committed
PerformanceInliner: protect against misuse of @inline(__always)
Inline-always should only be used on relatively small functions. It must not be used on recursive functions. Add a check that prevents that inlining of large @inline(__always) functions. #64319 rdar://106655649
1 parent d38e4e8 commit 83bc199

File tree

1 file changed

+34
-17
lines changed

1 file changed

+34
-17
lines changed

lib/SILOptimizer/Transforms/PerformanceInliner.cpp

Lines changed: 34 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -217,10 +217,10 @@ class SILPerformanceInliner {
217217
int &NumCallerBlocks,
218218
const llvm::DenseMap<SILBasicBlock *, uint64_t> &BBToWeightMap);
219219

220-
bool decideInColdBlock(FullApplySite AI, SILFunction *Callee);
220+
bool decideInColdBlock(FullApplySite AI, SILFunction *Callee, int numCallerBlocks);
221221

222222
void visitColdBlocks(SmallVectorImpl<FullApplySite> &AppliesToInline,
223-
SILBasicBlock *root, DominanceInfo *DT);
223+
SILBasicBlock *root, DominanceInfo *DT, int numCallerBlocks);
224224

225225
void collectAppliesToInline(SILFunction *Caller,
226226
SmallVectorImpl<FullApplySite> &Applies);
@@ -619,12 +619,29 @@ static bool returnsClosure(SILFunction *F) {
619619
return false;
620620
}
621621

622-
static bool isInlineAlwaysCallSite(SILFunction *Callee) {
622+
static bool hasMaxNumberOfBasicBlocks(SILFunction *f, int limit) {
623+
for (SILBasicBlock &block : *f) {
624+
(void)block;
625+
if (limit == 0)
626+
return false;
627+
limit--;
628+
}
629+
return true;
630+
}
631+
632+
static bool isInlineAlwaysCallSite(SILFunction *Callee, int numCallerBlocks) {
623633
if (Callee->isTransparent())
624634
return true;
625-
if (Callee->getInlineStrategy() == AlwaysInline)
626-
if (!Callee->getModule().getOptions().IgnoreAlwaysInline)
627-
return true;
635+
if (Callee->getInlineStrategy() == AlwaysInline &&
636+
!Callee->getModule().getOptions().IgnoreAlwaysInline &&
637+
638+
// Protect against misuse of @inline(__always).
639+
// Inline-always should only be used on relatively small functions.
640+
// It must not be used on recursive functions. This check prevents that
641+
// the compiler blows up if @inline(__always) is put on a recursive function.
642+
(numCallerBlocks < 64 || hasMaxNumberOfBasicBlocks(Callee, 64))) {
643+
return true;
644+
}
628645
return false;
629646
}
630647

@@ -634,7 +651,7 @@ static bool isInlineAlwaysCallSite(SILFunction *Callee) {
634651
/// It returns false if a function should not be inlined.
635652
/// It returns None if the decision cannot be made without a more complex
636653
/// analysis.
637-
static Optional<bool> shouldInlineGeneric(FullApplySite AI) {
654+
static Optional<bool> shouldInlineGeneric(FullApplySite AI, int numCallerBlocks) {
638655
assert(AI.hasSubstitutions() &&
639656
"Expected a generic apply");
640657

@@ -654,7 +671,7 @@ static Optional<bool> shouldInlineGeneric(FullApplySite AI) {
654671

655672
// Always inline generic functions which are marked as
656673
// AlwaysInline or transparent.
657-
if (isInlineAlwaysCallSite(Callee))
674+
if (isInlineAlwaysCallSite(Callee, numCallerBlocks))
658675
return true;
659676

660677
// If all substitutions are concrete, then there is no need to perform the
@@ -694,14 +711,14 @@ bool SILPerformanceInliner::decideInWarmBlock(
694711
const llvm::DenseMap<SILBasicBlock *, uint64_t> &BBToWeightMap) {
695712
if (AI.hasSubstitutions()) {
696713
// Only inline generics if definitively clear that it should be done.
697-
auto ShouldInlineGeneric = shouldInlineGeneric(AI);
714+
auto ShouldInlineGeneric = shouldInlineGeneric(AI, NumCallerBlocks);
698715
if (ShouldInlineGeneric.has_value())
699716
return ShouldInlineGeneric.value();
700717
}
701718

702719
SILFunction *Callee = AI.getReferencedFunctionOrNull();
703720

704-
if (isInlineAlwaysCallSite(Callee)) {
721+
if (isInlineAlwaysCallSite(Callee, NumCallerBlocks)) {
705722
LLVM_DEBUG(dumpCaller(AI.getFunction());
706723
llvm::dbgs() << " always-inline decision "
707724
<< Callee->getName() << '\n');
@@ -714,17 +731,17 @@ bool SILPerformanceInliner::decideInWarmBlock(
714731

715732
/// Return true if inlining this call site into a cold block is profitable.
716733
bool SILPerformanceInliner::decideInColdBlock(FullApplySite AI,
717-
SILFunction *Callee) {
734+
SILFunction *Callee, int numCallerBlocks) {
718735
if (AI.hasSubstitutions()) {
719736
// Only inline generics if definitively clear that it should be done.
720-
auto ShouldInlineGeneric = shouldInlineGeneric(AI);
737+
auto ShouldInlineGeneric = shouldInlineGeneric(AI, numCallerBlocks);
721738
if (ShouldInlineGeneric.has_value())
722739
return ShouldInlineGeneric.value();
723740

724741
return false;
725742
}
726743

727-
if (isInlineAlwaysCallSite(Callee)) {
744+
if (isInlineAlwaysCallSite(Callee, numCallerBlocks)) {
728745
LLVM_DEBUG(dumpCaller(AI.getFunction());
729746
llvm::dbgs() << " always-inline decision "
730747
<< Callee->getName() << '\n');
@@ -923,7 +940,7 @@ void SILPerformanceInliner::collectAppliesToInline(
923940
if (Callee) {
924941
// Check if we have an always_inline or transparent function. If we do,
925942
// just add it to our final Applies list and continue.
926-
if (isInlineAlwaysCallSite(Callee)) {
943+
if (isInlineAlwaysCallSite(Callee, NumCallerBlocks)) {
927944
NumCallerBlocks += Callee->size();
928945
Applies.push_back(AI);
929946
continue;
@@ -953,7 +970,7 @@ void SILPerformanceInliner::collectAppliesToInline(
953970
domOrder.pushChildrenIf(block, [&] (SILBasicBlock *child) {
954971
if (CBI.isSlowPath(block, child)) {
955972
// Handle cold blocks separately.
956-
visitColdBlocks(InitialCandidates, child, DT);
973+
visitColdBlocks(InitialCandidates, child, DT, NumCallerBlocks);
957974
return false;
958975
}
959976
return true;
@@ -1089,7 +1106,7 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
10891106
// All other functions are not inlined in cold blocks.
10901107
void SILPerformanceInliner::visitColdBlocks(
10911108
SmallVectorImpl<FullApplySite> &AppliesToInline, SILBasicBlock *Root,
1092-
DominanceInfo *DT) {
1109+
DominanceInfo *DT, int numCallerBlocks) {
10931110
DominanceOrder domOrder(Root, DT);
10941111
while (SILBasicBlock *block = domOrder.getNext()) {
10951112
for (SILInstruction &I : *block) {
@@ -1098,7 +1115,7 @@ void SILPerformanceInliner::visitColdBlocks(
10981115
continue;
10991116

11001117
auto *Callee = getEligibleFunction(AI, WhatToInline);
1101-
if (Callee && decideInColdBlock(AI, Callee)) {
1118+
if (Callee && decideInColdBlock(AI, Callee, numCallerBlocks)) {
11021119
AppliesToInline.push_back(AI);
11031120
}
11041121
}

0 commit comments

Comments
 (0)