AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Factor out Inliner candidate list assembly into its own function.

Browse Source 
This greatly improves the output from a profiler. It makes it much
easier to determine how much time is spent in searching for candidates,
versus actually inlining them.

It also improves the code readability somewhat by breaking a large
monolithic function into several smaller functions.

Change-Id: I1b3ef6ddbe46af60e673f37ded766f8077ed6b03
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321376
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-10-02 15:01:03 -04:00 committed by Skia Commit-Bot
parent 034f78a466
commit 2d7973afc2
5 changed files with 120 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/sksl/SkSLIRGenerator.cpp
View File

@ -2116,9 +2116,11 @@ std::unique_ptr<Expression> IRGenerator::call(int offset,
auto funcCall = std::make_unique<FunctionCall>(offset, returnType, function,
std::move(arguments));
if (fCanInline && fInliner->isSafeToInline(*funcCall, fSettings->fInlineThreshold)) {
Inliner::InlinedCall inlinedCall =
fInliner->inlineCall(funcCall.get(), fSymbolTable.get(), fCurrentFunction);
if (fCanInline &&
fInliner->isSafeToInline(funcCall->fFunction.fDefinition) &&
!fInliner->isLargeFunction(funcCall->fFunction.fDefinition)) {
Inliner::InlinedCall inlinedCall = fInliner->inlineCall(funcCall.get(), fSymbolTable.get(),
fCurrentFunction);
if (inlinedCall.fInlinedBody) {
fExtraStatements.push_back(std::move(inlinedCall.fInlinedBody));
}

155
src/sksl/SkSLInliner.cpp
View File

@ -7,7 +7,7 @@
#include "src/sksl/SkSLInliner.h"
#include "limits.h"
#include <limits.h>
#include <memory>
#include <unordered_set>
@ -592,7 +592,7 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
SkASSERT(fSettings);
SkASSERT(fContext);
SkASSERT(call);
SkASSERT(this->isSafeToInline(*call, /*inlineThreshold=*/INT_MAX));
SkASSERT(this->isSafeToInline(call->fFunction.fDefinition));
std::vector<std::unique_ptr<Expression>>& arguments = call->fArguments;
const int offset = call->fOffset;
@ -745,58 +745,52 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
return inlinedCall;
}
bool Inliner::isSafeToInline(const FunctionCall& functionCall, int inlineThreshold) {
bool Inliner::isSafeToInline(const FunctionDefinition* functionDef) {
SkASSERT(fSettings);
if (functionCall.fFunction.fDefinition == nullptr) {
if (functionDef == nullptr) {
// Can't inline something if we don't actually have its definition.
return false;
}
const FunctionDefinition& functionDef = *functionCall.fFunction.fDefinition;
if (inlineThreshold < INT_MAX) {
if (!(functionDef.fDeclaration.fModifiers.fFlags & Modifiers::kInline_Flag) &&
Analysis::NodeCountExceeds(functionDef, inlineThreshold)) {
// The function exceeds our maximum inline size and is not flagged 'inline'.
return false;
}
}
if (!fSettings->fCaps || !fSettings->fCaps->canUseDoLoops()) {
// We don't have do-while loops. We use do-while loops to simulate early returns, so we
// can't inline functions that have an early return.
bool hasEarlyReturn = has_early_return(functionDef);
bool hasEarlyReturn = has_early_return(*functionDef);
// If we didn't detect an early return, there shouldn't be any returns in breakable
// constructs either.
SkASSERT(hasEarlyReturn || count_returns_in_breakable_constructs(functionDef) == 0);
SkASSERT(hasEarlyReturn || count_returns_in_breakable_constructs(*functionDef) == 0);
return !hasEarlyReturn;
}
// We have do-while loops, but we don't have any mechanism to simulate early returns within a
// breakable construct (switch/for/do/while), so we can't inline if there's a return inside one.
bool hasReturnInBreakableConstruct = (count_returns_in_breakable_constructs(functionDef) > 0);
bool hasReturnInBreakableConstruct = (count_returns_in_breakable_constructs(*functionDef) > 0);
// If we detected returns in breakable constructs, we should also detect an early return.
SkASSERT(!hasReturnInBreakableConstruct || has_early_return(functionDef));
SkASSERT(!hasReturnInBreakableConstruct || has_early_return(*functionDef));
return !hasReturnInBreakableConstruct;
}
bool Inliner::analyze(Program& program) {
// A candidate function for inlining, containing everything that `inlineCall` needs.
struct InlineCandidate {
SymbolTable* fSymbols; // the SymbolTable of the candidate
std::unique_ptr<Statement>* fParentStmt; // the parent Statement of the enclosing stmt
std::unique_ptr<Statement>* fEnclosingStmt; // the Statement containing the candidate
std::unique_ptr<Expression>* fCandidateExpr; // the candidate FunctionCall to be inlined
FunctionDefinition* fEnclosingFunction; // the Function containing the candidate
};
// A candidate function for inlining, containing everything that `inlineCall` needs.
struct InlineCandidate {
SymbolTable* fSymbols; // the SymbolTable of the candidate
std::unique_ptr<Statement>* fParentStmt; // the parent Statement of the enclosing stmt
std::unique_ptr<Statement>* fEnclosingStmt; // the Statement containing the candidate
std::unique_ptr<Expression>* fCandidateExpr; // the candidate FunctionCall to be inlined
FunctionDefinition* fEnclosingFunction; // the Function containing the candidate
bool fIsLargeFunction; // does candidate exceed the inline threshold?
};
// This is structured much like a ProgramVisitor, but does not actually use ProgramVisitor.
// The analyzer needs to keep track of the `unique_ptr<T>*` of statements and expressions so
// that they can later be replaced, and ProgramVisitor does not provide this; it only provides a
// `const T&`.
class InlineCandidateAnalyzer {
struct InlineCandidateList {
std::vector<InlineCandidate> fCandidates;
};
class InlineCandidateAnalyzer {
public:
// A list of all the inlining candidates we found during analysis.
std::vector<InlineCandidate> fInlineCandidates;
InlineCandidateList* fCandidateList;
// A stack of the symbol tables; since most nodes don't have one, expected to be shallower
// than the enclosing-statement stack.
std::vector<SymbolTable*> fSymbolTableStack;
@ -807,7 +801,8 @@ bool Inliner::analyze(Program& program) {
// The function that we're currently processing (i.e. inlining into).
FunctionDefinition* fEnclosingFunction = nullptr;
void visit(Program& program) {
void visit(Program& program, InlineCandidateList* candidateList) {
fCandidateList = candidateList;
fSymbolTableStack.push_back(program.fSymbols.get());
for (ProgramElement& pe : program) {
@ -815,6 +810,7 @@ bool Inliner::analyze(Program& program) {
}
fSymbolTableStack.pop_back();
fCandidateList = nullptr;
}
void visitProgramElement(ProgramElement* pe) {
@ -1072,42 +1068,87 @@ bool Inliner::analyze(Program& program) {
}
void addInlineCandidate(std::unique_ptr<Expression>* candidate) {
fInlineCandidates.push_back(InlineCandidate{fSymbolTableStack.back(),
find_parent_statement(fEnclosingStmtStack),
fEnclosingStmtStack.back(),
candidate,
fEnclosingFunction});
fCandidateList->fCandidates.push_back(
InlineCandidate{fSymbolTableStack.back(),
find_parent_statement(fEnclosingStmtStack),
fEnclosingStmtStack.back(),
candidate,
fEnclosingFunction,
/*isLargeFunction=*/false});
}
};
};
InlineCandidateAnalyzer analyzer;
analyzer.visit(program);
bool Inliner::candidateCanBeInlined(const InlineCandidate& candidate, InlinabilityCache* cache) {
const FunctionDeclaration& funcDecl = (*candidate.fCandidateExpr)->as<FunctionCall>().fFunction;
// For each of our candidate function-call sites, check if it is actually safe to inline.
// Memoize our results so we don't check a function more than once.
std::unordered_map<const FunctionDeclaration*, bool> inlinableMap; // <function, safe-to-inline>
for (const InlineCandidate& candidate : analyzer.fInlineCandidates) {
const FunctionCall& funcCall = (*candidate.fCandidateExpr)->as<FunctionCall>();
const FunctionDeclaration* funcDecl = &funcCall.fFunction;
if (inlinableMap.find(funcDecl) == inlinableMap.end()) {
// We do not perform inlining on recursive calls to avoid an infinite death spiral of
// inlining.
int inlineThreshold = (funcDecl->fCallCount.load() > 1) ? fSettings->fInlineThreshold
: INT_MAX;
inlinableMap[funcDecl] = this->isSafeToInline(funcCall, inlineThreshold) &&
!contains_recursive_call(*funcDecl);
}
auto [iter, wasInserted] = cache->insert({&funcDecl, false});
if (wasInserted) {
// Recursion is forbidden here to avoid an infinite death spiral of inlining.
iter->second = this->isSafeToInline(funcDecl.fDefinition) &&
!contains_recursive_call(funcDecl);
}
return iter->second;
}
bool Inliner::isLargeFunction(const FunctionDefinition* functionDef) {
return Analysis::NodeCountExceeds(*functionDef, fSettings->fInlineThreshold);
}
bool Inliner::isLargeFunction(const InlineCandidate& candidate, LargeFunctionCache* cache) {
const FunctionDeclaration& funcDecl = (*candidate.fCandidateExpr)->as<FunctionCall>().fFunction;
auto [iter, wasInserted] = cache->insert({&funcDecl, false});
if (wasInserted) {
iter->second = this->isLargeFunction(funcDecl.fDefinition);
}
return iter->second;
}
void Inliner::buildCandidateList(Program& program, InlineCandidateList* candidateList) {
// This is structured much like a ProgramVisitor, but does not actually use ProgramVisitor.
// The analyzer needs to keep track of the `unique_ptr<T>*` of statements and expressions so
// that they can later be replaced, and ProgramVisitor does not provide this; it only provides a
// `const T&`.
InlineCandidateAnalyzer analyzer;
analyzer.visit(program, candidateList);
// Remove candidates that are not safe to inline.
std::vector<InlineCandidate>& candidates = candidateList->fCandidates;
InlinabilityCache cache;
candidates.erase(std::remove_if(candidates.begin(),
candidates.end(),
[&](const InlineCandidate& candidate) {
return !this->candidateCanBeInlined(candidate, &cache);
}),
candidates.end());
// Determine whether each candidate function exceeds our inlining size threshold or not. These
// can still be valid candidates if they are only called one time, so we don't remove them from
// the candidate list, but they will not be inlined if they're called more than once.
LargeFunctionCache largeFunctionCache;
for (InlineCandidate& candidate : candidates) {
candidate.fIsLargeFunction = this->isLargeFunction(candidate, &largeFunctionCache);
}
}
bool Inliner::analyze(Program& program) {
InlineCandidateList candidateList;
this->buildCandidateList(program, &candidateList);
// Inline the candidates where we've determined that it's safe to do so.
std::unordered_set<const std::unique_ptr<Statement>*> enclosingStmtSet;
bool madeChanges = false;
for (const InlineCandidate& candidate : analyzer.fInlineCandidates) {
for (const InlineCandidate& candidate : candidateList.fCandidates) {
FunctionCall& funcCall = (*candidate.fCandidateExpr)->as<FunctionCall>();
const FunctionDeclaration* funcDecl = &funcCall.fFunction;
// If we determined that this candidate was not actually inlinable, skip it.
if (!inlinableMap[funcDecl]) {
// If the function is large, not marked `inline`, and is called more than once, it's a bad
// idea to inline it.
if (candidate.fIsLargeFunction &&
!(funcDecl->fModifiers.fFlags & Modifiers::kInline_Flag) &&
funcDecl->fCallCount.load() > 1) {
continue;
}

18
src/sksl/SkSLInliner.h
View File

@ -20,6 +20,9 @@ class Block;
class Context;
struct Expression;
struct FunctionCall;
struct FunctionDefinition;
struct InlineCandidate;
struct InlineCandidateList;
struct Statement;
class SymbolTable;
struct Variable;
@ -50,8 +53,11 @@ public:
/** Adds a scope to inlined bodies returned by `inlineCall`, if one is required. */
void ensureScopedBlocks(Statement* inlinedBody, Statement* parentStmt);
/** Checks whether inlining is viable for a FunctionCall. */
bool isSafeToInline(const FunctionCall&, int inlineThreshold);
/** Checks whether inlining is viable for a FunctionCall, modulo recursion and function size. */
bool isSafeToInline(const FunctionDefinition* functionDef);
/** Checks whether a function's size exceeds the inline threshold from Settings. */
bool isLargeFunction(const FunctionDefinition* functionDef);
/** Inlines any eligible functions that are found. Returns true if any changes are made. */
bool analyze(Program& program);
@ -61,6 +67,8 @@ private:
String uniqueNameForInlineVar(const String& baseName, SymbolTable* symbolTable);
void buildCandidateList(Program& program, InlineCandidateList* candidateList);
std::unique_ptr<Expression> inlineExpression(int offset,
VariableRewriteMap* varMap,
const Expression& expression);
@ -72,6 +80,12 @@ private:
const Statement& statement,
bool isBuiltinCode);
using InlinabilityCache = std::unordered_map<const FunctionDeclaration*, bool>;
bool candidateCanBeInlined(const InlineCandidate& candidate, InlinabilityCache* cache);
using LargeFunctionCache = std::unordered_map<const FunctionDeclaration*, bool>;
bool isLargeFunction(const InlineCandidate& candidate, LargeFunctionCache* cache);
const Context* fContext = nullptr;
const Program::Settings* fSettings = nullptr;
int fInlineVarCounter = 0;

2
tests/sksl/inliner/golden/InlineKeywordOverridesThreshold.glsl
View File

@ -38,7 +38,6 @@ void main() {
++y;
}
{
++y;
++y;
@ -76,5 +75,4 @@ void main() {
++y;
}
}

3
tests/sksl/inliner/golden/InlineWithNestedBigCalls.glsl
View File

@ -40,7 +40,6 @@ void main() {
--_1_x;
_1_x = 456.0;
}
float _3_x = 456.0;
{
++_3_x;
@ -79,7 +78,7 @@ void main() {
--_3_x;
_3_x = 123.0;
}
sk_FragColor = vec4(123.0);
}