Fix indenting on InlineCandidateAnalyzer.

No code changes in this CL, only hundreds of lines of indentation fixes.

Change-Id: I780a0f93a61e567c4dca0e8b8d7066350569dc55
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/321795
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2020-10-02 16:42:10 -04:00 committed by Skia Commit-Bot
parent a6ca9757d3
commit 70957c8bc8

View File

@ -787,295 +787,293 @@ struct InlineCandidateList {
};
class InlineCandidateAnalyzer {
public:
// A list of all the inlining candidates we found during analysis.
InlineCandidateList* fCandidateList;
public:
// A list of all the inlining candidates we found during analysis.
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;
// A stack of "enclosing" statements--these would be suitable for the inliner to use for
// adding new instructions. Not all statements are suitable (e.g. a for-loop's initializer).
// The inliner might replace a statement with a block containing the statement.
std::vector<std::unique_ptr<Statement>*> fEnclosingStmtStack;
// The function that we're currently processing (i.e. inlining into).
FunctionDefinition* fEnclosingFunction = nullptr;
// 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;
// A stack of "enclosing" statements--these would be suitable for the inliner to use for adding
// new instructions. Not all statements are suitable (e.g. a for-loop's initializer). The
// inliner might replace a statement with a block containing the statement.
std::vector<std::unique_ptr<Statement>*> fEnclosingStmtStack;
// The function that we're currently processing (i.e. inlining into).
FunctionDefinition* fEnclosingFunction = nullptr;
void visit(Program& program, InlineCandidateList* candidateList) {
fCandidateList = candidateList;
fSymbolTableStack.push_back(program.fSymbols.get());
void visit(Program& program, InlineCandidateList* candidateList) {
fCandidateList = candidateList;
fSymbolTableStack.push_back(program.fSymbols.get());
for (ProgramElement& pe : program) {
this->visitProgramElement(&pe);
}
fSymbolTableStack.pop_back();
fCandidateList = nullptr;
for (ProgramElement& pe : program) {
this->visitProgramElement(&pe);
}
void visitProgramElement(ProgramElement* pe) {
switch (pe->kind()) {
case ProgramElement::Kind::kFunction: {
FunctionDefinition& funcDef = pe->as<FunctionDefinition>();
fEnclosingFunction = &funcDef;
this->visitStatement(&funcDef.fBody);
break;
}
default:
// The inliner can't operate outside of a function's scope.
break;
fSymbolTableStack.pop_back();
fCandidateList = nullptr;
}
void visitProgramElement(ProgramElement* pe) {
switch (pe->kind()) {
case ProgramElement::Kind::kFunction: {
FunctionDefinition& funcDef = pe->as<FunctionDefinition>();
fEnclosingFunction = &funcDef;
this->visitStatement(&funcDef.fBody);
break;
}
default:
// The inliner can't operate outside of a function's scope.
break;
}
}
void visitStatement(std::unique_ptr<Statement>* stmt,
bool isViableAsEnclosingStatement = true) {
if (!*stmt) {
return;
}
void visitStatement(std::unique_ptr<Statement>* stmt,
bool isViableAsEnclosingStatement = true) {
if (!*stmt) {
return;
}
size_t oldEnclosingStmtStackSize = fEnclosingStmtStack.size();
size_t oldSymbolStackSize = fSymbolTableStack.size();
size_t oldEnclosingStmtStackSize = fEnclosingStmtStack.size();
size_t oldSymbolStackSize = fSymbolTableStack.size();
if (isViableAsEnclosingStatement) {
fEnclosingStmtStack.push_back(stmt);
}
switch ((*stmt)->kind()) {
case Statement::Kind::kBreak:
case Statement::Kind::kContinue:
case Statement::Kind::kDiscard:
case Statement::Kind::kInlineMarker:
case Statement::Kind::kNop:
break;
case Statement::Kind::kBlock: {
Block& block = (*stmt)->as<Block>();
if (block.symbolTable()) {
fSymbolTableStack.push_back(block.symbolTable().get());
}
for (std::unique_ptr<Statement>& stmt : block.children()) {
this->visitStatement(&stmt);
}
break;
}
case Statement::Kind::kDo: {
DoStatement& doStmt = (*stmt)->as<DoStatement>();
// The loop body is a candidate for inlining.
this->visitStatement(&doStmt.statement());
// The inliner isn't smart enough to inline the test-expression for a do-while
// loop at this time. There are two limitations:
// - We would need to insert the inlined-body block at the very end of the do-
// statement's inner fStatement. We don't support that today, but it's doable.
// - We cannot inline the test expression if the loop uses `continue` anywhere;
// that would skip over the inlined block that evaluates the test expression.
// There isn't a good fix for this--any workaround would be more complex than
// the cost of a function call. However, loops that don't use `continue` would
// still be viable candidates for inlining.
break;
}
case Statement::Kind::kExpression: {
ExpressionStatement& expr = (*stmt)->as<ExpressionStatement>();
this->visitExpression(&expr.expression());
break;
}
case Statement::Kind::kFor: {
ForStatement& forStmt = (*stmt)->as<ForStatement>();
if (forStmt.fSymbols) {
fSymbolTableStack.push_back(forStmt.fSymbols.get());
}
// The initializer and loop body are candidates for inlining.
this->visitStatement(&forStmt.fInitializer,
/*isViableAsEnclosingStatement=*/false);
this->visitStatement(&forStmt.fStatement);
// The inliner isn't smart enough to inline the test- or increment-expressions
// of a for loop loop at this time. There are a handful of limitations:
// - We would need to insert the test-expression block at the very beginning of
// the for-loop's inner fStatement, and the increment-expression block at the
// very end. We don't support that today, but it's doable.
// - The for-loop's built-in test-expression would need to be dropped entirely,
// and the loop would be halted via a break statement at the end of the
// inlined test-expression. This is again something we don't support today,
// but it could be implemented.
// - We cannot inline the increment-expression if the loop uses `continue`
// anywhere; that would skip over the inlined block that evaluates the
// increment expression. There isn't a good fix for this--any workaround would
// be more complex than the cost of a function call. However, loops that don't
// use `continue` would still be viable candidates for increment-expression
// inlining.
break;
}
case Statement::Kind::kIf: {
IfStatement& ifStmt = (*stmt)->as<IfStatement>();
this->visitExpression(&ifStmt.fTest);
this->visitStatement(&ifStmt.fIfTrue);
this->visitStatement(&ifStmt.fIfFalse);
break;
}
case Statement::Kind::kReturn: {
ReturnStatement& returnStmt = (*stmt)->as<ReturnStatement>();
this->visitExpression(&returnStmt.fExpression);
break;
}
case Statement::Kind::kSwitch: {
SwitchStatement& switchStmt = (*stmt)->as<SwitchStatement>();
if (switchStmt.fSymbols) {
fSymbolTableStack.push_back(switchStmt.fSymbols.get());
}
this->visitExpression(&switchStmt.fValue);
for (std::unique_ptr<SwitchCase>& switchCase : switchStmt.fCases) {
// The switch-case's fValue cannot be a FunctionCall; skip it.
for (std::unique_ptr<Statement>& caseBlock : switchCase->fStatements) {
this->visitStatement(&caseBlock);
}
}
break;
}
case Statement::Kind::kVarDeclaration: {
VarDeclaration& varDeclStmt = (*stmt)->as<VarDeclaration>();
// Don't need to scan the declaration's sizes; those are always IntLiterals.
this->visitExpression(&varDeclStmt.fValue);
break;
}
case Statement::Kind::kVarDeclarations: {
VarDeclarationsStatement& varDecls = (*stmt)->as<VarDeclarationsStatement>();
for (std::unique_ptr<Statement>& varDecl : varDecls.fDeclaration->fVars) {
this->visitStatement(&varDecl, /*isViableAsEnclosingStatement=*/false);
}
break;
}
case Statement::Kind::kWhile: {
WhileStatement& whileStmt = (*stmt)->as<WhileStatement>();
// The loop body is a candidate for inlining.
this->visitStatement(&whileStmt.fStatement);
// The inliner isn't smart enough to inline the test-expression for a while
// loop at this time. There are two limitations:
// - We would need to insert the inlined-body block at the very beginning of the
// while loop's inner fStatement. We don't support that today, but it's
// doable.
// - The while-loop's built-in test-expression would need to be replaced with a
// `true` BoolLiteral, and the loop would be halted via a break statement at
// the end of the inlined test-expression. This is again something we don't
// support today, but it could be implemented.
break;
}
default:
SkUNREACHABLE;
}
// Pop our symbol and enclosing-statement stacks.
fSymbolTableStack.resize(oldSymbolStackSize);
fEnclosingStmtStack.resize(oldEnclosingStmtStackSize);
if (isViableAsEnclosingStatement) {
fEnclosingStmtStack.push_back(stmt);
}
void visitExpression(std::unique_ptr<Expression>* expr) {
if (!*expr) {
return;
switch ((*stmt)->kind()) {
case Statement::Kind::kBreak:
case Statement::Kind::kContinue:
case Statement::Kind::kDiscard:
case Statement::Kind::kInlineMarker:
case Statement::Kind::kNop:
break;
case Statement::Kind::kBlock: {
Block& block = (*stmt)->as<Block>();
if (block.symbolTable()) {
fSymbolTableStack.push_back(block.symbolTable().get());
}
for (std::unique_ptr<Statement>& stmt : block.children()) {
this->visitStatement(&stmt);
}
break;
}
switch ((*expr)->kind()) {
case Expression::Kind::kBoolLiteral:
case Expression::Kind::kDefined:
case Expression::Kind::kExternalValue:
case Expression::Kind::kFieldAccess:
case Expression::Kind::kFloatLiteral:
case Expression::Kind::kFunctionReference:
case Expression::Kind::kIntLiteral:
case Expression::Kind::kNullLiteral:
case Expression::Kind::kSetting:
case Expression::Kind::kTypeReference:
case Expression::Kind::kVariableReference:
// Nothing to scan here.
break;
case Expression::Kind::kBinary: {
BinaryExpression& binaryExpr = (*expr)->as<BinaryExpression>();
this->visitExpression(&binaryExpr.leftPointer());
// Logical-and and logical-or binary expressions do not inline the right side,
// because that would invalidate short-circuiting. That is, when evaluating
// expressions like these:
// (false && x()) // always false
// (true || y()) // always true
// It is illegal for side-effects from x() or y() to occur. The simplest way to
// enforce that rule is to avoid inlining the right side entirely. However, it
// is safe for other types of binary expression to inline both sides.
Token::Kind op = binaryExpr.getOperator();
bool shortCircuitable = (op == Token::Kind::TK_LOGICALAND ||
op == Token::Kind::TK_LOGICALOR);
if (!shortCircuitable) {
this->visitExpression(&binaryExpr.rightPointer());
}
break;
}
case Expression::Kind::kConstructor: {
Constructor& constructorExpr = (*expr)->as<Constructor>();
for (std::unique_ptr<Expression>& arg : constructorExpr.arguments()) {
this->visitExpression(&arg);
}
break;
}
case Expression::Kind::kExternalFunctionCall: {
ExternalFunctionCall& funcCallExpr = (*expr)->as<ExternalFunctionCall>();
for (std::unique_ptr<Expression>& arg : funcCallExpr.arguments()) {
this->visitExpression(&arg);
}
break;
}
case Expression::Kind::kFunctionCall: {
FunctionCall& funcCallExpr = (*expr)->as<FunctionCall>();
for (std::unique_ptr<Expression>& arg : funcCallExpr.fArguments) {
this->visitExpression(&arg);
}
this->addInlineCandidate(expr);
break;
}
case Expression::Kind::kIndex:{
IndexExpression& indexExpr = (*expr)->as<IndexExpression>();
this->visitExpression(&indexExpr.fBase);
this->visitExpression(&indexExpr.fIndex);
break;
}
case Expression::Kind::kPostfix: {
PostfixExpression& postfixExpr = (*expr)->as<PostfixExpression>();
this->visitExpression(&postfixExpr.fOperand);
break;
}
case Expression::Kind::kPrefix: {
PrefixExpression& prefixExpr = (*expr)->as<PrefixExpression>();
this->visitExpression(&prefixExpr.fOperand);
break;
}
case Expression::Kind::kSwizzle: {
Swizzle& swizzleExpr = (*expr)->as<Swizzle>();
this->visitExpression(&swizzleExpr.fBase);
break;
}
case Expression::Kind::kTernary: {
TernaryExpression& ternaryExpr = (*expr)->as<TernaryExpression>();
// The test expression is a candidate for inlining.
this->visitExpression(&ternaryExpr.fTest);
// The true- and false-expressions cannot be inlined, because we are only
// allowed to evaluate one side.
break;
}
default:
SkUNREACHABLE;
case Statement::Kind::kDo: {
DoStatement& doStmt = (*stmt)->as<DoStatement>();
// The loop body is a candidate for inlining.
this->visitStatement(&doStmt.statement());
// The inliner isn't smart enough to inline the test-expression for a do-while
// loop at this time. There are two limitations:
// - We would need to insert the inlined-body block at the very end of the do-
// statement's inner fStatement. We don't support that today, but it's doable.
// - We cannot inline the test expression if the loop uses `continue` anywhere; that
// would skip over the inlined block that evaluates the test expression. There
// isn't a good fix for this--any workaround would be more complex than the cost
// of a function call. However, loops that don't use `continue` would still be
// viable candidates for inlining.
break;
}
case Statement::Kind::kExpression: {
ExpressionStatement& expr = (*stmt)->as<ExpressionStatement>();
this->visitExpression(&expr.expression());
break;
}
case Statement::Kind::kFor: {
ForStatement& forStmt = (*stmt)->as<ForStatement>();
if (forStmt.fSymbols) {
fSymbolTableStack.push_back(forStmt.fSymbols.get());
}
// The initializer and loop body are candidates for inlining.
this->visitStatement(&forStmt.fInitializer,
/*isViableAsEnclosingStatement=*/false);
this->visitStatement(&forStmt.fStatement);
// The inliner isn't smart enough to inline the test- or increment-expressions
// of a for loop loop at this time. There are a handful of limitations:
// - We would need to insert the test-expression block at the very beginning of the
// for-loop's inner fStatement, and the increment-expression block at the very
// end. We don't support that today, but it's doable.
// - The for-loop's built-in test-expression would need to be dropped entirely,
// and the loop would be halted via a break statement at the end of the inlined
// test-expression. This is again something we don't support today, but it could
// be implemented.
// - We cannot inline the increment-expression if the loop uses `continue` anywhere;
// that would skip over the inlined block that evaluates the increment expression.
// There isn't a good fix for this--any workaround would be more complex than the
// cost of a function call. However, loops that don't use `continue` would still
// be viable candidates for increment-expression inlining.
break;
}
case Statement::Kind::kIf: {
IfStatement& ifStmt = (*stmt)->as<IfStatement>();
this->visitExpression(&ifStmt.fTest);
this->visitStatement(&ifStmt.fIfTrue);
this->visitStatement(&ifStmt.fIfFalse);
break;
}
case Statement::Kind::kReturn: {
ReturnStatement& returnStmt = (*stmt)->as<ReturnStatement>();
this->visitExpression(&returnStmt.fExpression);
break;
}
case Statement::Kind::kSwitch: {
SwitchStatement& switchStmt = (*stmt)->as<SwitchStatement>();
if (switchStmt.fSymbols) {
fSymbolTableStack.push_back(switchStmt.fSymbols.get());
}
this->visitExpression(&switchStmt.fValue);
for (std::unique_ptr<SwitchCase>& switchCase : switchStmt.fCases) {
// The switch-case's fValue cannot be a FunctionCall; skip it.
for (std::unique_ptr<Statement>& caseBlock : switchCase->fStatements) {
this->visitStatement(&caseBlock);
}
}
break;
}
case Statement::Kind::kVarDeclaration: {
VarDeclaration& varDeclStmt = (*stmt)->as<VarDeclaration>();
// Don't need to scan the declaration's sizes; those are always IntLiterals.
this->visitExpression(&varDeclStmt.fValue);
break;
}
case Statement::Kind::kVarDeclarations: {
VarDeclarationsStatement& varDecls = (*stmt)->as<VarDeclarationsStatement>();
for (std::unique_ptr<Statement>& varDecl : varDecls.fDeclaration->fVars) {
this->visitStatement(&varDecl, /*isViableAsEnclosingStatement=*/false);
}
break;
}
case Statement::Kind::kWhile: {
WhileStatement& whileStmt = (*stmt)->as<WhileStatement>();
// The loop body is a candidate for inlining.
this->visitStatement(&whileStmt.fStatement);
// The inliner isn't smart enough to inline the test-expression for a while loop at
// this time. There are two limitations:
// - We would need to insert the inlined-body block at the very beginning of the
// while loop's inner fStatement. We don't support that today, but it's doable.
// - The while-loop's built-in test-expression would need to be replaced with a
// `true` BoolLiteral, and the loop would be halted via a break statement at the
// end of the inlined test-expression. This is again something we don't support
// today, but it could be implemented.
break;
}
default:
SkUNREACHABLE;
}
void addInlineCandidate(std::unique_ptr<Expression>* candidate) {
fCandidateList->fCandidates.push_back(
InlineCandidate{fSymbolTableStack.back(),
find_parent_statement(fEnclosingStmtStack),
fEnclosingStmtStack.back(),
candidate,
fEnclosingFunction,
/*isLargeFunction=*/false});
// Pop our symbol and enclosing-statement stacks.
fSymbolTableStack.resize(oldSymbolStackSize);
fEnclosingStmtStack.resize(oldEnclosingStmtStackSize);
}
void visitExpression(std::unique_ptr<Expression>* expr) {
if (!*expr) {
return;
}
switch ((*expr)->kind()) {
case Expression::Kind::kBoolLiteral:
case Expression::Kind::kDefined:
case Expression::Kind::kExternalValue:
case Expression::Kind::kFieldAccess:
case Expression::Kind::kFloatLiteral:
case Expression::Kind::kFunctionReference:
case Expression::Kind::kIntLiteral:
case Expression::Kind::kNullLiteral:
case Expression::Kind::kSetting:
case Expression::Kind::kTypeReference:
case Expression::Kind::kVariableReference:
// Nothing to scan here.
break;
case Expression::Kind::kBinary: {
BinaryExpression& binaryExpr = (*expr)->as<BinaryExpression>();
this->visitExpression(&binaryExpr.leftPointer());
// Logical-and and logical-or binary expressions do not inline the right side,
// because that would invalidate short-circuiting. That is, when evaluating
// expressions like these:
// (false && x()) // always false
// (true || y()) // always true
// It is illegal for side-effects from x() or y() to occur. The simplest way to
// enforce that rule is to avoid inlining the right side entirely. However, it is
// safe for other types of binary expression to inline both sides.
Token::Kind op = binaryExpr.getOperator();
bool shortCircuitable = (op == Token::Kind::TK_LOGICALAND ||
op == Token::Kind::TK_LOGICALOR);
if (!shortCircuitable) {
this->visitExpression(&binaryExpr.rightPointer());
}
break;
}
case Expression::Kind::kConstructor: {
Constructor& constructorExpr = (*expr)->as<Constructor>();
for (std::unique_ptr<Expression>& arg : constructorExpr.arguments()) {
this->visitExpression(&arg);
}
break;
}
case Expression::Kind::kExternalFunctionCall: {
ExternalFunctionCall& funcCallExpr = (*expr)->as<ExternalFunctionCall>();
for (std::unique_ptr<Expression>& arg : funcCallExpr.arguments()) {
this->visitExpression(&arg);
}
break;
}
case Expression::Kind::kFunctionCall: {
FunctionCall& funcCallExpr = (*expr)->as<FunctionCall>();
for (std::unique_ptr<Expression>& arg : funcCallExpr.fArguments) {
this->visitExpression(&arg);
}
this->addInlineCandidate(expr);
break;
}
case Expression::Kind::kIndex:{
IndexExpression& indexExpr = (*expr)->as<IndexExpression>();
this->visitExpression(&indexExpr.fBase);
this->visitExpression(&indexExpr.fIndex);
break;
}
case Expression::Kind::kPostfix: {
PostfixExpression& postfixExpr = (*expr)->as<PostfixExpression>();
this->visitExpression(&postfixExpr.fOperand);
break;
}
case Expression::Kind::kPrefix: {
PrefixExpression& prefixExpr = (*expr)->as<PrefixExpression>();
this->visitExpression(&prefixExpr.fOperand);
break;
}
case Expression::Kind::kSwizzle: {
Swizzle& swizzleExpr = (*expr)->as<Swizzle>();
this->visitExpression(&swizzleExpr.fBase);
break;
}
case Expression::Kind::kTernary: {
TernaryExpression& ternaryExpr = (*expr)->as<TernaryExpression>();
// The test expression is a candidate for inlining.
this->visitExpression(&ternaryExpr.fTest);
// The true- and false-expressions cannot be inlined, because we are only allowed to
// evaluate one side.
break;
}
default:
SkUNREACHABLE;
}
}
void addInlineCandidate(std::unique_ptr<Expression>* candidate) {
fCandidateList->fCandidates.push_back(
InlineCandidate{fSymbolTableStack.back(),
find_parent_statement(fEnclosingStmtStack),
fEnclosingStmtStack.back(),
candidate,
fEnclosingFunction,
/*isLargeFunction=*/false});
}
};
bool Inliner::candidateCanBeInlined(const InlineCandidate& candidate, InlinabilityCache* cache) {