Replace inliner do-while loops with for loops.
do-while loops aren't compatible with GLSL ES2. For-loops which run
only one time should work exactly the same for our purposes. We expect
such a loop to be unrolled by every driver, so it shouldn't come at any
performance cost.
Change-Id: Ia8de5fcab8128c34da97eaeaf81f91ad1ac36ce4
Bug: skia:11097
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/345159
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
index 8e2e402..3950d00 100644
--- a/src/sksl/SkSLInliner.cpp
+++ b/src/sksl/SkSLInliner.cpp
@@ -520,15 +520,15 @@
StatementArray block;
block.reserve_back(2);
block.push_back(std::move(assignment));
- block.push_back(std::make_unique<BreakStatement>(offset));
- return std::make_unique<Block>(offset, std::move(block), /*symbols=*/nullptr,
- /*isScope=*/true);
+ block.push_back(std::make_unique<ContinueStatement>(offset));
+ return std::make_unique<Block>(offset, std::move(block),
+ /*symbols=*/nullptr, /*isScope=*/true);
} else {
return std::move(assignment);
}
} else {
if (haveEarlyReturns) {
- return std::make_unique<BreakStatement>(offset);
+ return std::make_unique<ContinueStatement>(offset);
} else {
return std::make_unique<Nop>();
}
@@ -577,6 +577,51 @@
}
}
+Inliner::InlineVariable Inliner::makeInlineVariable(const String& baseName,
+ const Type* type,
+ SymbolTable* symbolTable,
+ Modifiers modifiers,
+ bool isBuiltinCode,
+ std::unique_ptr<Expression>* initialValue) {
+ // $floatLiteral or $intLiteral aren't real types that we can use for scratch variables, so
+ // replace them if they ever appear here. If this happens, we likely forgot to coerce a type
+ // somewhere during compilation.
+ if (type == fContext->fFloatLiteral_Type.get()) {
+ SkDEBUGFAIL("found a $floatLiteral type while inlining");
+ type = fContext->fFloat_Type.get();
+ } else if (type == fContext->fIntLiteral_Type.get()) {
+ SkDEBUGFAIL("found an $intLiteral type while inlining");
+ type = fContext->fInt_Type.get();
+ }
+
+ // Provide our new variable with a unique name, and add it to our symbol table.
+ const String* namePtr = symbolTable->takeOwnershipOfString(
+ std::make_unique<String>(this->uniqueNameForInlineVar(baseName, symbolTable)));
+ StringFragment nameFrag{namePtr->c_str(), namePtr->length()};
+
+ // Create our new variable and add it to the symbol table.
+ InlineVariable result;
+ result.fVarSymbol =
+ symbolTable->add(std::make_unique<Variable>(/*offset=*/-1,
+ fModifiers->addToPool(Modifiers()),
+ nameFrag,
+ type,
+ isBuiltinCode,
+ Variable::Storage::kLocal,
+ initialValue->get()));
+
+ // Prepare the variable declaration (taking extra care with `out` params to not clobber any
+ // initial value).
+ if (*initialValue && (modifiers.fFlags & Modifiers::kOut_Flag)) {
+ result.fVarDecl = std::make_unique<VarDeclaration>(result.fVarSymbol, type, /*arraySize=*/0,
+ (*initialValue)->clone());
+ } else {
+ result.fVarDecl = std::make_unique<VarDeclaration>(result.fVarSymbol, type, /*arraySize=*/0,
+ std::move(*initialValue));
+ }
+ return result;
+}
+
Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
std::shared_ptr<SymbolTable> symbolTable,
const FunctionDeclaration* caller) {
@@ -610,59 +655,20 @@
1 + // Result variable
arguments.size() + // Function arguments (passing in)
arguments.size() + // Function arguments (copy out-params back)
- 1); // Inlined code (Block or do-while loop)
+ 1); // Block for inlined code
inlinedBody.children().push_back(std::make_unique<InlineMarker>(&call->function()));
- auto makeInlineVar =
- [&](const String& baseName, const Type* type, Modifiers modifiers,
- std::unique_ptr<Expression>* initialValue) -> std::unique_ptr<Expression> {
- // $floatLiteral or $intLiteral aren't real types that we can use for scratch variables, so
- // replace them if they ever appear here. If this happens, we likely forgot to coerce a type
- // somewhere during compilation.
- if (type == fContext->fFloatLiteral_Type.get()) {
- SkDEBUGFAIL("found a $floatLiteral type while inlining");
- type = fContext->fFloat_Type.get();
- } else if (type == fContext->fIntLiteral_Type.get()) {
- SkDEBUGFAIL("found an $intLiteral type while inlining");
- type = fContext->fInt_Type.get();
- }
-
- // Provide our new variable with a unique name, and add it to our symbol table.
- const String* namePtr = symbolTable->takeOwnershipOfString(std::make_unique<String>(
- this->uniqueNameForInlineVar(baseName, symbolTable.get())));
- StringFragment nameFrag{namePtr->c_str(), namePtr->length()};
-
- // Add our new variable to the symbol table.
- const Variable* variableSymbol = symbolTable->add(std::make_unique<Variable>(
- /*offset=*/-1, fModifiers->addToPool(Modifiers()),
- nameFrag, type, caller->isBuiltin(),
- Variable::Storage::kLocal, initialValue->get()));
-
- // Prepare the variable declaration (taking extra care with `out` params to not clobber any
- // initial value).
- std::unique_ptr<Statement> variable;
- if (initialValue && (modifiers.fFlags & Modifiers::kOut_Flag)) {
- variable = std::make_unique<VarDeclaration>(variableSymbol, type, /*arraySize=*/0,
- (*initialValue)->clone());
- } else {
- variable = std::make_unique<VarDeclaration>(variableSymbol, type, /*arraySize=*/0,
- std::move(*initialValue));
- }
-
- // Add the new variable-declaration statement to our block of extra statements.
- inlinedBody.children().push_back(std::move(variable));
-
- return std::make_unique<VariableReference>(offset, variableSymbol);
- };
-
// Create a variable to hold the result in the extra statements (excepting void).
std::unique_ptr<Expression> resultExpr;
if (function.declaration().returnType() != *fContext->fVoid_Type) {
std::unique_ptr<Expression> noInitialValue;
- resultExpr = makeInlineVar(String(function.declaration().name()),
- &function.declaration().returnType(),
- Modifiers{}, &noInitialValue);
+ InlineVariable var = this->makeInlineVariable(function.declaration().name(),
+ &function.declaration().returnType(),
+ symbolTable.get(), Modifiers{},
+ caller->isBuiltin(), &noInitialValue);
+ inlinedBody.children().push_back(std::move(var.fVarDecl));
+ resultExpr = std::make_unique<VariableReference>(/*offset=*/-1, var.fVarSymbol);
}
// Create variables in the extra statements to hold the arguments, and assign the arguments to
@@ -682,44 +688,81 @@
continue;
}
}
-
if (isOutParam) {
argsToCopyBack.push_back(i);
}
-
- varMap[param] = makeInlineVar(String(param->name()), &arguments[i]->type(),
- param->modifiers(), &arguments[i]);
+ InlineVariable var = this->makeInlineVariable(param->name(), &arguments[i]->type(),
+ symbolTable.get(), param->modifiers(),
+ caller->isBuiltin(), &arguments[i]);
+ inlinedBody.children().push_back(std::move(var.fVarDecl));
+ varMap[param] = std::make_unique<VariableReference>(/*offset=*/-1, var.fVarSymbol);
}
const Block& body = function.body()->as<Block>();
- auto inlineBlock = std::make_unique<Block>(offset, StatementArray{},
- /*symbols=*/nullptr, /*isScope=*/hasEarlyReturn);
- inlineBlock->children().reserve_back(body.children().size());
- for (const std::unique_ptr<Statement>& stmt : body.children()) {
- inlineBlock->children().push_back(this->inlineStatement(offset, &varMap, symbolTable.get(),
- resultExpr.get(), hasEarlyReturn,
- *stmt, caller->isBuiltin()));
- }
+ StatementArray* inlineStatements;
+
if (hasEarlyReturn) {
// Since we output to backends that don't have a goto statement (which would normally be
- // used to perform an early return), we fake it by wrapping the function in a
- // do { } while (false); and then use break statements to jump to the end in order to
- // emulate a goto.
- inlinedBody.children().push_back(std::make_unique<DoStatement>(
+ // used to perform an early return), we fake it by wrapping the function in a single-
+ // iteration for loop, and use a continue statement to jump to the end of the loop
+ // prematurely.
+
+ // int _1_loop = 0;
+ symbolTable = std::make_shared<SymbolTable>(std::move(symbolTable), caller->isBuiltin());
+ const Type* intType = fContext->fInt_Type.get();
+ std::unique_ptr<Expression> initialValue = std::make_unique<IntLiteral>(/*offset=*/-1,
+ /*value=*/0,
+ intType);
+ InlineVariable loopVar = this->makeInlineVariable("loop", intType, symbolTable.get(),
+ Modifiers{}, caller->isBuiltin(),
+ &initialValue);
+
+ // _1_loop < 1;
+ std::unique_ptr<Expression> test = std::make_unique<BinaryExpression>(
/*offset=*/-1,
- std::move(inlineBlock),
- std::make_unique<BoolLiteral>(*fContext, offset, /*value=*/false)));
+ std::make_unique<VariableReference>(/*offset=*/-1, loopVar.fVarSymbol),
+ Token::Kind::TK_LT,
+ std::make_unique<IntLiteral>(/*offset=*/-1, /*value=*/1, intType),
+ fContext->fBool_Type.get());
+
+ // _1_loop++
+ std::unique_ptr<Expression> increment = std::make_unique<PostfixExpression>(
+ std::make_unique<VariableReference>(/*offset=*/-1, loopVar.fVarSymbol,
+ VariableReference::RefKind::kReadWrite),
+ Token::Kind::TK_PLUSPLUS);
+
+ // {...}
+ auto innerBlock = std::make_unique<Block>(offset, StatementArray{},
+ /*symbols=*/nullptr, /*isScope=*/true);
+ inlineStatements = &innerBlock->children();
+
+ // for (int _1_loop = 0; _1_loop < 1; _1_loop++) {...}
+ inlinedBody.children().push_back(std::make_unique<ForStatement>(/*offset=*/-1,
+ std::move(loopVar.fVarDecl),
+ std::move(test),
+ std::move(increment),
+ std::move(innerBlock),
+ symbolTable));
} else {
- // No early returns, so we can just dump the code in. We still need to keep the block so we
- // don't get name conflicts with locals.
- inlinedBody.children().push_back(std::move(inlineBlock));
+ // No early returns, so we can just dump the code into a scopeless block.
+ auto innerBlock = std::make_unique<Block>(offset, StatementArray{},
+ /*symbols=*/nullptr, /*isScope=*/false);
+ inlineStatements = &innerBlock->children();
+ inlinedBody.children().push_back(std::move(innerBlock));
+ }
+
+ inlineStatements->reserve_back(body.children().size() + argsToCopyBack.size());
+ for (const std::unique_ptr<Statement>& stmt : body.children()) {
+ inlineStatements->push_back(this->inlineStatement(offset, &varMap, symbolTable.get(),
+ resultExpr.get(), hasEarlyReturn, *stmt,
+ caller->isBuiltin()));
}
// Copy back the values of `out` parameters into their real destinations.
for (int i : argsToCopyBack) {
const Variable* p = function.declaration().parameters()[i];
SkASSERT(varMap.find(p) != varMap.end());
- inlinedBody.children().push_back(
+ inlineStatements->push_back(
std::make_unique<ExpressionStatement>(std::make_unique<BinaryExpression>(
offset,
clone_with_ref_kind(*arguments[i], VariableReference::RefKind::kWrite),
@@ -762,18 +805,8 @@
return false;
}
- if (!fCaps || !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);
-
- // 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);
- 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.
+ // 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);
// If we detected returns in breakable constructs, we should also detect an early return.