Skip to content

PerformanceInliner: protect against misuse of @inline(__always) #64635

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 28, 2023
Merged

PerformanceInliner: protect against misuse of @inline(__always) #64635

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3497248
PassManager: add an option `-sil-min-pass-time` to only print long ru…
eeckstein Mar 27, 2023
0a4b0a8
PerformanceInliner: protect against misuse of @inline(__always)
eeckstein Mar 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions lib/SILOptimizer/PassManager/PassManager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,10 @@ llvm::cl::opt<bool> SILPrintPassTime(
"sil-print-pass-time", llvm::cl::init(false),
llvm::cl::desc("Print the execution time of each SIL pass"));

llvm::cl::opt<unsigned> SILMinPassTime(
"sil-min-pass-time", llvm::cl::init(0),
llvm::cl::desc("The minimum number of milliseconds for which a pass is printed with -sil-print-pass-time"));

llvm::cl::opt<bool> SILPrintLast(
"sil-print-last", llvm::cl::init(false),
llvm::cl::desc("Print the last optimized function before and after the last pass"));
Expand Down Expand Up @@ -619,8 +623,10 @@ void SILPassManager::runPassOnFunction(unsigned TransIdx, SILFunction *F) {
totalPassRuntime += duration;
if (SILPrintPassTime) {
double milliSecs = (double)duration.count() / 1000000.;
llvm::dbgs() << llvm::format("%9.3f", milliSecs) << " ms: " << SFT->getTag()
<< " @" << F->getName() << "\n";
if (milliSecs > (double)SILMinPassTime) {
llvm::dbgs() << llvm::format("%9.3f", milliSecs) << " ms: " << SFT->getTag()
<< " #" << NumPassesRun << " @" << F->getName() << "\n";
}
}

if (numRepeats > 1)
Expand Down Expand Up @@ -783,7 +789,10 @@ void SILPassManager::runModulePass(unsigned TransIdx) {

if (SILPrintPassTime) {
double milliSecs = (double)duration.count() / 1000000.;
llvm::dbgs() << llvm::format("%9.3f", milliSecs) << " ms: " << SMT->getTag() << "\n";
if (milliSecs > (double)SILMinPassTime) {
llvm::dbgs() << llvm::format("%9.3f", milliSecs) << " ms: " << SMT->getTag()
<< " #" << NumPassesRun << "\n";
}
}

// If this pass invalidated anything, print and verify.
Expand Down
51 changes: 34 additions & 17 deletions lib/SILOptimizer/Transforms/PerformanceInliner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,10 +217,10 @@ class SILPerformanceInliner {
int &NumCallerBlocks,
const llvm::DenseMap<SILBasicBlock *, uint64_t> &BBToWeightMap);

bool decideInColdBlock(FullApplySite AI, SILFunction *Callee);
bool decideInColdBlock(FullApplySite AI, SILFunction *Callee, int numCallerBlocks);

void visitColdBlocks(SmallVectorImpl<FullApplySite> &AppliesToInline,
SILBasicBlock *root, DominanceInfo *DT);
SILBasicBlock *root, DominanceInfo *DT, int numCallerBlocks);

void collectAppliesToInline(SILFunction *Caller,
SmallVectorImpl<FullApplySite> &Applies);
Expand Down Expand Up @@ -619,12 +619,29 @@ static bool returnsClosure(SILFunction *F) {
return false;
}

static bool isInlineAlwaysCallSite(SILFunction *Callee) {
static bool hasMaxNumberOfBasicBlocks(SILFunction *f, int limit) {
for (SILBasicBlock &block : *f) {
(void)block;
if (limit == 0)
return false;
limit--;
}
return true;
}

static bool isInlineAlwaysCallSite(SILFunction *Callee, int numCallerBlocks) {
if (Callee->isTransparent())
return true;
if (Callee->getInlineStrategy() == AlwaysInline)
if (!Callee->getModule().getOptions().IgnoreAlwaysInline)
return true;
if (Callee->getInlineStrategy() == AlwaysInline &&
!Callee->getModule().getOptions().IgnoreAlwaysInline &&

// Protect against misuse of @inline(__always).
// Inline-always should only be used on relatively small functions.
// It must not be used on recursive functions. This check prevents that
// the compiler blows up if @inline(__always) is put on a recursive function.
(numCallerBlocks < 64 || hasMaxNumberOfBasicBlocks(Callee, 64))) {
return true;
}
return false;
}

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

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

// Always inline generic functions which are marked as
// AlwaysInline or transparent.
if (isInlineAlwaysCallSite(Callee))
if (isInlineAlwaysCallSite(Callee, numCallerBlocks))
return true;

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

SILFunction *Callee = AI.getReferencedFunctionOrNull();

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

/// Return true if inlining this call site into a cold block is profitable.
bool SILPerformanceInliner::decideInColdBlock(FullApplySite AI,
SILFunction *Callee) {
SILFunction *Callee, int numCallerBlocks) {
if (AI.hasSubstitutions()) {
// Only inline generics if definitively clear that it should be done.
auto ShouldInlineGeneric = shouldInlineGeneric(AI);
auto ShouldInlineGeneric = shouldInlineGeneric(AI, numCallerBlocks);
if (ShouldInlineGeneric.has_value())
return ShouldInlineGeneric.value();

return false;
}

if (isInlineAlwaysCallSite(Callee)) {
if (isInlineAlwaysCallSite(Callee, numCallerBlocks)) {
LLVM_DEBUG(dumpCaller(AI.getFunction());
llvm::dbgs() << " always-inline decision "
<< Callee->getName() << '\n');
Expand Down Expand Up @@ -923,7 +940,7 @@ void SILPerformanceInliner::collectAppliesToInline(
if (Callee) {
// Check if we have an always_inline or transparent function. If we do,
// just add it to our final Applies list and continue.
if (isInlineAlwaysCallSite(Callee)) {
if (isInlineAlwaysCallSite(Callee, NumCallerBlocks)) {
NumCallerBlocks += Callee->size();
Applies.push_back(AI);
continue;
Expand Down Expand Up @@ -953,7 +970,7 @@ void SILPerformanceInliner::collectAppliesToInline(
domOrder.pushChildrenIf(block, [&] (SILBasicBlock *child) {
if (CBI.isSlowPath(block, child)) {
// Handle cold blocks separately.
visitColdBlocks(InitialCandidates, child, DT);
visitColdBlocks(InitialCandidates, child, DT, NumCallerBlocks);
return false;
}
return true;
Expand Down Expand Up @@ -1089,7 +1106,7 @@ bool SILPerformanceInliner::inlineCallsIntoFunction(SILFunction *Caller) {
// All other functions are not inlined in cold blocks.
void SILPerformanceInliner::visitColdBlocks(
SmallVectorImpl<FullApplySite> &AppliesToInline, SILBasicBlock *Root,
DominanceInfo *DT) {
DominanceInfo *DT, int numCallerBlocks) {
DominanceOrder domOrder(Root, DT);
while (SILBasicBlock *block = domOrder.getNext()) {
for (SILInstruction &I : *block) {
Expand All @@ -1098,7 +1115,7 @@ void SILPerformanceInliner::visitColdBlocks(
continue;

auto *Callee = getEligibleFunction(AI, WhatToInline);
if (Callee && decideInColdBlock(AI, Callee)) {
if (Callee && decideInColdBlock(AI, Callee, numCallerBlocks)) {
AppliesToInline.push_back(AI);
}
}
Expand Down