Reland "Revert "SkSL function inlining""
This reverts commit 2dd272bf157de3d3edeff0fbc1a5bcc17eec3164.
Reason for revert: breaking angle
Original change's description:
> Revert "Revert "SkSL function inlining""
>
> This reverts commit 1b63b4ac6933ccddb15bcf650cd5747d5eba44d0.
>
> Change-Id: I8120bb10cecc6889f4f4fd7b4c3a61d250e49219
> Reviewed-on: https://skia-review.googlesource.com/c/skia/+/291358
> Reviewed-by: Brian Osman <brianosman@google.com>
> Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
TBR=bsalomon@google.com,brianosman@google.com,ethannicholas@google.com
# Not skipping CQ checks because this is a reland.
Change-Id: Ib3117efd1b77e97899e636bcbc4d84200118bc36
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/292264
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>
diff --git a/src/gpu/GrShaderCaps.cpp b/src/gpu/GrShaderCaps.cpp
index bab63ab..faafc94 100644
--- a/src/gpu/GrShaderCaps.cpp
+++ b/src/gpu/GrShaderCaps.cpp
@@ -61,7 +61,6 @@
// GL_ARB_texture_swizzle).
fTextureSwizzleAppliedInShader = true;
fBuiltinFMASupport = false;
- fCanInlineEarlyReturns = true;
fVersionDeclString = nullptr;
fShaderDerivativeExtensionString = nullptr;
@@ -143,7 +142,6 @@
writer->appendBool("Color space math needs float", fColorSpaceMathNeedsFloat);
writer->appendBool("Texture swizzle applied in shader", fTextureSwizzleAppliedInShader);
writer->appendBool("Builtin fma() support", fBuiltinFMASupport);
- writer->appendBool("Can inline early returns", fCanInlineEarlyReturns);
writer->appendS32("Max FS Samplers", fMaxFragmentSamplers);
writer->appendString("Advanced blend equation interaction",
diff --git a/src/gpu/GrShaderCaps.h b/src/gpu/GrShaderCaps.h
index 72d0f4b..b6a3cc0 100644
--- a/src/gpu/GrShaderCaps.h
+++ b/src/gpu/GrShaderCaps.h
@@ -175,10 +175,6 @@
// http://skbug.com/8921
bool canOnlyUseSampleMaskWithStencil() const { return fCanOnlyUseSampleMaskWithStencil; }
- // Seeing crashes on Tegra3 with inlined functions that have early returns. Looks like the
- // do { ... break; } while (false); construct is causing a crash in the driver.
- bool canInlineEarlyReturns() const { return fCanInlineEarlyReturns; }
-
// Returns the string of an extension that must be enabled in the shader to support
// derivatives. If nullptr is returned then no extension needs to be enabled. Before calling
// this function, the caller should check that shaderDerivativeSupport exists.
@@ -305,7 +301,6 @@
bool fNoDefaultPrecisionForExternalSamplers : 1;
bool fCanOnlyUseSampleMaskWithStencil : 1;
bool fColorSpaceMathNeedsFloat : 1;
- bool fCanInlineEarlyReturns : 1;
const char* fVersionDeclString;
diff --git a/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp b/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
index 0abe9fb..94d81a0 100644
--- a/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
+++ b/src/gpu/effects/generated/GrColorMatrixFragmentProcessor.cpp
@@ -39,12 +39,9 @@
vVar = args.fUniformHandler->addUniform(&_outer, kFragment_GrShaderFlag, kHalf4_GrSLType,
"v");
fragBuilder->codeAppendf(
- "half4 inputColor = %s;\n@if (%s) {\n half4 inlineResult530;\n half4 "
- "inlineArg530_0 = inputColor;\n {\n inlineResult530 = "
- "half4(inlineArg530_0.xyz / max(inlineArg530_0.w, 9.9999997473787516e-05), "
- "inlineArg530_0.w);\n }\n inputColor = inlineResult530;\n\n}\n%s = %s * "
- "inputColor + %s;\n@if (%s) {\n %s = clamp(%s, 0.0, 1.0);\n} else {\n %s.w = "
- "clamp(%s.w, 0.0, 1.0);\n}\n@if (%s) {\n %s.xyz *= %s.w;\n}\n",
+ "half4 inputColor = %s;\n@if (%s) {\n inputColor = unpremul(inputColor);\n}\n%s "
+ "= %s * inputColor + %s;\n@if (%s) {\n %s = clamp(%s, 0.0, 1.0);\n} else {\n "
+ "%s.w = clamp(%s.w, 0.0, 1.0);\n}\n@if (%s) {\n %s.xyz *= %s.w;\n}\n",
args.fInputColor, (_outer.unpremulInput ? "true" : "false"), args.fOutputColor,
args.fUniformHandler->getUniformCStr(mVar),
args.fUniformHandler->getUniformCStr(vVar),
diff --git a/src/gpu/gl/GrGLCaps.cpp b/src/gpu/gl/GrGLCaps.cpp
index 6eee20e..79ffe09 100644
--- a/src/gpu/gl/GrGLCaps.cpp
+++ b/src/gpu/gl/GrGLCaps.cpp
@@ -3645,10 +3645,6 @@
// Tegra3 fract() seems to trigger undefined behavior for negative values, so we
// must avoid this condition.
shaderCaps->fCanUseFractForNegativeValues = false;
-
- // Seeing crashes on Tegra3 with inlined functions that have early returns. Looks like the
- // do { ... break; } while (false); construct is causing a crash in the driver.
- shaderCaps->fCanInlineEarlyReturns = false;
}
// On Intel GPU there is an issue where it reads the second argument to atan "- %s.x" as an int
diff --git a/src/sksl/SkSLASTNode.cpp b/src/sksl/SkSLASTNode.cpp
index a74d5ab..7c5a203 100644
--- a/src/sksl/SkSLASTNode.cpp
+++ b/src/sksl/SkSLASTNode.cpp
@@ -34,7 +34,7 @@
return "break";
case Kind::kCall: {
auto iter = this->begin();
- String result = (iter++)->description();
+ String result = iter->description();
result += "(";
const char* separator = "";
while (iter != this->end()) {
@@ -230,11 +230,6 @@
}
return result;
}
- case Kind::kWhile: {
- return "while (" + this->begin()->description() + ") " +
- (this->begin() + 1)->description();
-
- }
default:
SkASSERT(false);
return "<error>";
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 85f4bb6..fe75fe7 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -1269,14 +1269,6 @@
break;
case BasicBlock::Node::kExpression_Kind:
offset = (*cfg.fBlocks[i].fNodes[0].expression())->fOffset;
- if ((*cfg.fBlocks[i].fNodes[0].expression())->fKind ==
- Expression::kBoolLiteral_Kind) {
- // Function inlining can generate do { ... } while(false) loops which always
- // break, so the boolean condition is considered unreachable. Since not
- // being able to reach a literal is a non-issue in the first place, we
- // don't report an error in this case.
- continue;
- }
break;
}
this->error(offset, String("unreachable"));
diff --git a/src/sksl/SkSLContext.h b/src/sksl/SkSLContext.h
index 39968d4..eb7659a 100644
--- a/src/sksl/SkSLContext.h
+++ b/src/sksl/SkSLContext.h
@@ -400,11 +400,6 @@
return "<defined>";
}
- int nodeCount() const override {
- SkASSERT(false);
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new Defined(fType));
}
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 0aa7bbc..e21c32c 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -1521,15 +1521,11 @@
}
void GLSLCodeGenerator::writeBlock(const Block& b) {
- if (b.fIsScope) {
- this->writeLine("{");
- fIndentation++;
- }
+ this->writeLine("{");
+ fIndentation++;
this->writeStatements(b.fStatements);
- if (b.fIsScope) {
- fIndentation--;
- this->write("}");
- }
+ fIndentation--;
+ this->write("}");
}
void GLSLCodeGenerator::writeIfStatement(const IfStatement& stmt) {
diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp
index 91a5788..8f2b1c5 100644
--- a/src/sksl/SkSLIRGenerator.cpp
+++ b/src/sksl/SkSLIRGenerator.cpp
@@ -182,7 +182,7 @@
fSettings = nullptr;
}
-std::unique_ptr<Statement> IRGenerator::convertSingleStatement(const ASTNode& statement) {
+std::unique_ptr<Statement> IRGenerator::convertStatement(const ASTNode& statement) {
switch (statement.fKind) {
case ASTNode::Kind::kBlock:
return this->convertBlock(statement);
@@ -228,24 +228,6 @@
}
}
-std::unique_ptr<Statement> IRGenerator::convertStatement(const ASTNode& statement) {
- std::vector<std::unique_ptr<Statement>> oldExtraStatements = std::move(fExtraStatements);
- std::unique_ptr<Statement> result = this->convertSingleStatement(statement);
- if (!result) {
- fExtraStatements = std::move(oldExtraStatements);
- return nullptr;
- }
- if (fExtraStatements.size()) {
- fExtraStatements.push_back(std::move(result));
- std::unique_ptr<Statement> block(new Block(-1, std::move(fExtraStatements), nullptr,
- false));
- fExtraStatements = std::move(oldExtraStatements);
- return block;
- }
- fExtraStatements = std::move(oldExtraStatements);
- return result;
-}
-
std::unique_ptr<Block> IRGenerator::convertBlock(const ASTNode& block) {
SkASSERT(block.fKind == ASTNode::Kind::kBlock);
AutoSymbolTable table(this);
@@ -514,22 +496,15 @@
++iter;
std::unique_ptr<Expression> test;
if (*iter) {
- bool oldCanInline = fCanInline;
- fCanInline = false;
test = this->coerce(this->convertExpression(*iter), *fContext.fBool_Type);
- fCanInline = oldCanInline;
if (!test) {
return nullptr;
}
-
}
++iter;
std::unique_ptr<Expression> next;
if (*iter) {
- bool oldCanInline = fCanInline;
- fCanInline = false;
next = this->convertExpression(*iter);
- fCanInline = oldCanInline;
if (!next) {
return nullptr;
}
@@ -549,11 +524,8 @@
SkASSERT(w.fKind == ASTNode::Kind::kWhile);
AutoLoopLevel level(this);
auto iter = w.begin();
- bool oldCanInline = fCanInline;
- fCanInline = false;
std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
*fContext.fBool_Type);
- fCanInline = oldCanInline;
if (!test) {
return nullptr;
}
@@ -573,11 +545,8 @@
if (!statement) {
return nullptr;
}
- bool oldCanInline = fCanInline;
- fCanInline = false;
std::unique_ptr<Expression> test = this->coerce(this->convertExpression(*(iter++)),
*fContext.fBool_Type);
- fCanInline = oldCanInline;
if (!test) {
return nullptr;
}
@@ -915,7 +884,7 @@
return;
}
}
- if (other->fDefinition && !other->fBuiltin) {
+ if (other->fDefined && !other->fBuiltin) {
fErrors.error(f.fOffset, "duplicate definition of " +
other->declaration());
}
@@ -938,6 +907,7 @@
// compile body
SkASSERT(!fCurrentFunction);
fCurrentFunction = decl;
+ decl->fDefined = true;
std::shared_ptr<SymbolTable> old = fSymbolTable;
AutoSymbolTable table(this);
if (fd.fName == "main" && fKind == Program::kPipelineStage_Kind) {
@@ -970,7 +940,6 @@
}
std::unique_ptr<FunctionDefinition> result(new FunctionDefinition(f.fOffset, *decl,
std::move(body)));
- decl->fDefinition = result.get();
result->fSource = &f;
fProgramElements->push_back(std::move(result));
}
@@ -1746,15 +1715,7 @@
if (!left) {
return nullptr;
}
- Token::Kind op = expression.getToken().fKind;
- bool oldCanInline = fCanInline;
- if (op == Token::Kind::TK_LOGICALAND || op == Token::Kind::TK_LOGICALOR) {
- // can't inline the right side of a short-circuiting boolean, because our inlining
- // approach runs things out of order
- fCanInline = false;
- }
std::unique_ptr<Expression> right = this->convertExpression(*(iter++));
- fCanInline = oldCanInline;
if (!right) {
return nullptr;
}
@@ -1773,6 +1734,7 @@
} else {
rawRightType = &right->fType;
}
+ Token::Kind op = expression.getToken().fKind;
if (!determine_binary_type(fContext, op, *rawLeftType, *rawRightType, &leftType, &rightType,
&resultType, !Compiler::IsAssignment(op))) {
fErrors.error(expression.fOffset, String("type mismatch: '") +
@@ -1849,416 +1811,6 @@
std::move(ifFalse)));
}
-std::unique_ptr<Expression> IRGenerator::inlineExpression(int offset,
- std::map<const Variable*,
- const Variable*>* varMap,
- const Expression& expression) {
- auto expr = [&](const std::unique_ptr<Expression>& e) {
- if (e) {
- return this->inlineExpression(offset, varMap, *e);
- }
- return std::unique_ptr<Expression>(nullptr);
- };
- switch (expression.fKind) {
- case Expression::kBinary_Kind: {
- const BinaryExpression& b = (const BinaryExpression&) expression;
- return std::unique_ptr<Expression>(new BinaryExpression(offset,
- expr(b.fLeft),
- b.fOperator,
- expr(b.fRight),
- b.fType));
- }
- case Expression::kBoolLiteral_Kind:
- case Expression::kIntLiteral_Kind:
- case Expression::kFloatLiteral_Kind:
- case Expression::kNullLiteral_Kind:
- return expression.clone();
- case Expression::kConstructor_Kind: {
- const Constructor& c = (const Constructor&) expression;
- std::vector<std::unique_ptr<Expression>> args;
- for (const auto& arg : c.fArguments) {
- args.push_back(expr(arg));
- }
- return std::unique_ptr<Expression>(new Constructor(offset, c.fType, std::move(args)));
- }
- case Expression::kExternalFunctionCall_Kind: {
- const ExternalFunctionCall& e = (const ExternalFunctionCall&) expression;
- std::vector<std::unique_ptr<Expression>> args;
- for (const auto& arg : e.fArguments) {
- args.push_back(expr(arg));
- }
- return std::unique_ptr<Expression>(new ExternalFunctionCall(offset, e.fType,
- e.fFunction,
- std::move(args)));
- }
- case Expression::kExternalValue_Kind:
- return expression.clone();
- case Expression::kFieldAccess_Kind: {
- const FieldAccess& f = (const FieldAccess&) expression;
- return std::unique_ptr<Expression>(new FieldAccess(expr(f.fBase), f.fFieldIndex,
- f.fOwnerKind));
- }
- case Expression::kFunctionCall_Kind: {
- const FunctionCall& c = (const FunctionCall&) expression;
- std::vector<std::unique_ptr<Expression>> args;
- for (const auto& arg : c.fArguments) {
- args.push_back(expr(arg));
- }
- return std::unique_ptr<Expression>(new FunctionCall(offset, c.fType, c.fFunction,
- std::move(args)));
- }
- case Expression::kIndex_Kind: {
- const IndexExpression& idx = (const IndexExpression&) expression;
- return std::unique_ptr<Expression>(new IndexExpression(fContext, expr(idx.fBase),
- expr(idx.fIndex)));
- }
- case Expression::kPrefix_Kind: {
- const PrefixExpression& p = (const PrefixExpression&) expression;
- return std::unique_ptr<Expression>(new PrefixExpression(p.fOperator, expr(p.fOperand)));
- }
- case Expression::kPostfix_Kind: {
- const PostfixExpression& p = (const PostfixExpression&) expression;
- return std::unique_ptr<Expression>(new PostfixExpression(expr(p.fOperand),
- p.fOperator));
- }
- case Expression::kSetting_Kind:
- return expression.clone();
- case Expression::kSwizzle_Kind: {
- const Swizzle& s = (const Swizzle&) expression;
- return std::unique_ptr<Expression>(new Swizzle(fContext, expr(s.fBase), s.fComponents));
- }
- case Expression::kTernary_Kind: {
- const TernaryExpression& t = (const TernaryExpression&) expression;
- return std::unique_ptr<Expression>(new TernaryExpression(offset, expr(t.fTest),
- expr(t.fIfTrue),
- expr(t.fIfFalse)));
- }
- case Expression::kVariableReference_Kind: {
- const VariableReference& v = (const VariableReference&) expression;
- auto found = varMap->find(&v.fVariable);
- if (found != varMap->end()) {
- return std::unique_ptr<Expression>(new VariableReference(offset,
- *found->second,
- v.fRefKind));
- }
- return v.clone();
- }
- default:
- SkASSERT(false);
- return nullptr;
- }
-}
-
-std::unique_ptr<Statement> IRGenerator::inlineStatement(int offset,
- std::map<const Variable*,
- const Variable*>* varMap,
- const Variable* returnVar,
- bool haveEarlyReturns,
- const Statement& statement) {
- auto stmt = [&](const std::unique_ptr<Statement>& s) {
- if (s) {
- return this->inlineStatement(offset, varMap, returnVar, haveEarlyReturns, *s);
- }
- return std::unique_ptr<Statement>(nullptr);
- };
- auto stmts = [&](const std::vector<std::unique_ptr<Statement>>& ss) {
- std::vector<std::unique_ptr<Statement>> result;
- for (const auto& s : ss) {
- result.push_back(stmt(s));
- }
- return result;
- };
- auto expr = [&](const std::unique_ptr<Expression>& e) {
- if (e) {
- return this->inlineExpression(offset, varMap, *e);
- }
- return std::unique_ptr<Expression>(nullptr);
- };
- switch (statement.fKind) {
- case Statement::kBlock_Kind: {
- const Block& b = (const Block&) statement;
- return std::unique_ptr<Statement>(new Block(offset, stmts(b.fStatements), b.fSymbols,
- b.fIsScope));
- }
-
- case Statement::kBreak_Kind:
- case Statement::kContinue_Kind:
- case Statement::kDiscard_Kind:
- return statement.clone();
-
- case Statement::kDo_Kind: {
- const DoStatement& d = (const DoStatement&) statement;
- return std::unique_ptr<Statement>(new DoStatement(offset,
- stmt(d.fStatement),
- expr(d.fTest)));
- }
- case Statement::kExpression_Kind: {
- const ExpressionStatement& e = (const ExpressionStatement&) statement;
- return std::unique_ptr<Statement>(new ExpressionStatement(expr(e.fExpression)));
- }
- case Statement::kFor_Kind: {
- const ForStatement& f = (const ForStatement&) statement;
- // need to ensure initializer is evaluated first so that we've already remapped its
- // declarations by the time we evaluate test & next
- std::unique_ptr<Statement> initializer = stmt(f.fInitializer);
- return std::unique_ptr<Statement>(new ForStatement(offset, std::move(initializer),
- expr(f.fTest), expr(f.fNext),
- stmt(f.fStatement), f.fSymbols));
- }
- case Statement::kIf_Kind: {
- const IfStatement& i = (const IfStatement&) statement;
- return std::unique_ptr<Statement>(new IfStatement(offset, i.fIsStatic, expr(i.fTest),
- stmt(i.fIfTrue), stmt(i.fIfFalse)));
- }
- case Statement::kNop_Kind:
- return statement.clone();
- case Statement::kReturn_Kind: {
- const ReturnStatement& r = (const ReturnStatement&) statement;
- if (r.fExpression) {
- std::unique_ptr<Statement> assignment(new ExpressionStatement(
- std::unique_ptr<Expression>(new BinaryExpression(offset,
- std::unique_ptr<Expression>(new VariableReference(
- offset,
- *returnVar,
- VariableReference::kWrite_RefKind)),
- Token::Kind::TK_EQ,
- expr(r.fExpression),
- returnVar->fType))));
- if (haveEarlyReturns) {
- std::vector<std::unique_ptr<Statement>> block;
- block.push_back(std::move(assignment));
- block.emplace_back(new BreakStatement(offset));
- return std::unique_ptr<Statement>(new Block(offset, std::move(block), nullptr,
- false));
- } else {
- return assignment;
- }
- } else {
- if (haveEarlyReturns) {
- return std::unique_ptr<Statement>(new BreakStatement(offset));
- } else {
- return std::unique_ptr<Statement>(new Nop());
- }
- }
- }
- case Statement::kSwitch_Kind: {
- const SwitchStatement& ss = (const SwitchStatement&) statement;
- std::vector<std::unique_ptr<SwitchCase>> cases;
- for (const auto& sc : ss.fCases) {
- cases.emplace_back(new SwitchCase(offset, expr(sc->fValue),
- stmts(sc->fStatements)));
- }
- return std::unique_ptr<Statement>(new SwitchStatement(offset, ss.fIsStatic,
- expr(ss.fValue),
- std::move(cases),
- ss.fSymbols));
- }
- case Statement::kVarDeclaration_Kind: {
- const VarDeclaration& decl = (const VarDeclaration&) statement;
- std::vector<std::unique_ptr<Expression>> sizes;
- for (const auto& size : decl.fSizes) {
- sizes.push_back(expr(size));
- }
- std::unique_ptr<Expression> initialValue = expr(decl.fValue);
- const Variable* old = decl.fVar;
- Variable* clone = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
- new Variable(offset, old->fModifiers, old->fName,
- old->fType, old->fStorage,
- initialValue.get())));
- (*varMap)[old] = clone;
- return std::unique_ptr<Statement>(new VarDeclaration(clone, std::move(sizes),
- std::move(initialValue)));
- }
- case Statement::kVarDeclarations_Kind: {
- const VarDeclarations& decls = *((VarDeclarationsStatement&) statement).fDeclaration;
- std::vector<std::unique_ptr<VarDeclaration>> vars;
- for (const auto& var : decls.fVars) {
- vars.emplace_back((VarDeclaration*) stmt(var).release());
- }
- return std::unique_ptr<Statement>(new VarDeclarationsStatement(
- std::unique_ptr<VarDeclarations>(new VarDeclarations(offset, &decls.fBaseType,
- std::move(vars)))));
- }
- case Statement::kWhile_Kind: {
- const WhileStatement& w = (const WhileStatement&) statement;
- return std::unique_ptr<Statement>(new WhileStatement(offset,
- expr(w.fTest),
- stmt(w.fStatement)));
- }
- default:
- SkASSERT(false);
- return nullptr;
- }
-}
-
-int return_count(const Statement& statement) {
- switch (statement.fKind) {
- case Statement::kBlock_Kind: {
- const Block& b = (const Block&) statement;
- int result = 0;
- for (const auto& s : b.fStatements) {
- result += return_count(*s);
- }
- return result;
- }
- case Statement::kDo_Kind: {
- const DoStatement& d = (const DoStatement&) statement;
- return return_count(*d.fStatement);
- }
- case Statement::kFor_Kind: {
- const ForStatement& f = (const ForStatement&) statement;
- return return_count(*f.fStatement);
- }
- case Statement::kIf_Kind: {
- const IfStatement& i = (const IfStatement&) statement;
- int result = return_count(*i.fIfTrue);
- if (i.fIfFalse) {
- result += return_count(*i.fIfFalse);
- }
- return result;
- }
- case Statement::kReturn_Kind:
- return 1;
- case Statement::kSwitch_Kind: {
- const SwitchStatement& ss = (const SwitchStatement&) statement;
- int result = 0;
- for (const auto& sc : ss.fCases) {
- for (const auto& s : ((SwitchCase&) *sc).fStatements) {
- result += return_count(*s);
- }
- }
- return result;
- }
- case Statement::kWhile_Kind: {
- const WhileStatement& w = (const WhileStatement&) statement;
- return return_count(*w.fStatement);
- }
- case Statement::kBreak_Kind:
- case Statement::kContinue_Kind:
- case Statement::kDiscard_Kind:
- case Statement::kExpression_Kind:
- case Statement::kNop_Kind:
- case Statement::kVarDeclaration_Kind:
- case Statement::kVarDeclarations_Kind:
- return 0;
- default:
- SkASSERT(false);
- return 0;
- }
-}
-
-bool has_early_return(const FunctionDefinition& f) {
- int returnCount = return_count(*f.fBody);
- if (returnCount == 0) {
- return false;
- }
- if (returnCount > 1) {
- return true;
- }
- SkASSERT(f.fBody->fKind == Statement::kBlock_Kind);
- return ((Block&) *f.fBody).fStatements.back()->fKind != Statement::kReturn_Kind;
-}
-
-std::unique_ptr<Expression> IRGenerator::inlineCall(
- int offset,
- const FunctionDefinition& function,
- std::vector<std::unique_ptr<Expression>> arguments) {
- // Inlining is more complicated here than in a typical compiler, because we have to have a
- // high-level IR and can't just drop statements into the middle of an expression or even use
- // gotos.
- //
- // Since we can't insert statements into an expression, we run the inline function as extra
- // statements before the statement we're currently processing, relying on a lack of execution
- // order guarantees. Since we can't use gotos (which are normally used to replace return
- // statements), we wrap the whole function in a loop and use break statements to jump to the
- // end.
- Variable* resultVar;
- if (function.fDeclaration.fReturnType != *fContext.fVoid_Type) {
- std::unique_ptr<String> name(new String());
- name->appendf("inlineResult%d", offset);
- String* namePtr = (String*) fSymbolTable->takeOwnership(std::move(name));
- resultVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
- new Variable(-1, Modifiers(), namePtr->c_str(),
- function.fDeclaration.fReturnType,
- Variable::kLocal_Storage,
- nullptr)));
- std::vector<std::unique_ptr<VarDeclaration>> variables;
- variables.emplace_back(new VarDeclaration(resultVar, {}, nullptr));
- fExtraStatements.emplace_back(new VarDeclarationsStatement(
- std::unique_ptr<VarDeclarations>(new VarDeclarations(offset,
- &resultVar->fType,
- std::move(variables)))));
-
- } else {
- resultVar = nullptr;
- }
- std::map<const Variable*, const Variable*> varMap;
- // create variables to hold the arguments and assign the arguments to them
- for (int i = 0; i < (int) arguments.size(); ++i) {
- std::unique_ptr<String> argName(new String());
- argName->appendf("inlineArg%d_%d", offset, i);
- String* argNamePtr = (String*) fSymbolTable->takeOwnership(std::move(argName));
- Variable* argVar = (Variable*) fSymbolTable->takeOwnership(std::unique_ptr<Symbol>(
- new Variable(-1, Modifiers(),
- argNamePtr->c_str(),
- arguments[i]->fType,
- Variable::kLocal_Storage,
- arguments[i].get())));
- varMap[function.fDeclaration.fParameters[i]] = argVar;
- std::vector<std::unique_ptr<VarDeclaration>> vars;
- if (function.fDeclaration.fParameters[i]->fModifiers.fFlags & Modifiers::kOut_Flag) {
- vars.emplace_back(new VarDeclaration(argVar, {}, arguments[i]->clone()));
- } else {
- vars.emplace_back(new VarDeclaration(argVar, {}, std::move(arguments[i])));
- }
- fExtraStatements.emplace_back(new VarDeclarationsStatement(
- std::unique_ptr<VarDeclarations>(new VarDeclarations(offset,
- &argVar->fType,
- std::move(vars)))));
- }
- SkASSERT(function.fBody->fKind == Statement::kBlock_Kind);
- const Block& body = (Block&) *function.fBody;
- bool hasEarlyReturn = has_early_return(function);
- std::vector<std::unique_ptr<Statement>> inlined;
- for (const auto& s : body.fStatements) {
- inlined.push_back(this->inlineStatement(offset, &varMap, resultVar, hasEarlyReturn, *s));
- }
- 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.
- fExtraStatements.emplace_back(new DoStatement(-1,
- std::unique_ptr<Statement>(new Block(-1, std::move(inlined))),
- std::unique_ptr<Expression>(new BoolLiteral(fContext, -1, false))));
- } else {
- // No early returns, so we can just dump the code in. We need to use a block so we don't get
- // name conflicts with locals.
- fExtraStatements.emplace_back(std::unique_ptr<Statement>(new Block(-1,
- std::move(inlined))));
- }
- // copy the values of out parameters into their destinations
- for (size_t i = 0; i < arguments.size(); ++i) {
- const Variable* p = function.fDeclaration.fParameters[i];
- if (p->fModifiers.fFlags & Modifiers::kOut_Flag) {
- std::unique_ptr<Expression> varRef(new VariableReference(offset, *varMap[p]));
- fExtraStatements.emplace_back(new ExpressionStatement(
- std::unique_ptr<Expression>(new BinaryExpression(offset,
- arguments[i]->clone(),
- Token::Kind::TK_EQ,
- std::move(varRef),
- arguments[i]->fType))));
- }
- }
- if (function.fDeclaration.fReturnType != *fContext.fVoid_Type) {
- return std::unique_ptr<Expression>(new VariableReference(-1, *resultVar));
- } else {
- // it's a void function, so it doesn't actually result in anything, but we have to return
- // something non-null as a standin
- return std::unique_ptr<Expression>(new BoolLiteral(fContext, -1, false));
- }
-}
-
std::unique_ptr<Expression> IRGenerator::call(int offset,
const FunctionDeclaration& function,
std::vector<std::unique_ptr<Expression>> arguments) {
@@ -2283,7 +1835,7 @@
fErrors.error(offset, msg);
return nullptr;
}
- if (fKind == Program::kPipelineStage_Kind && !function.fDefinition && !function.fBuiltin) {
+ if (fKind == Program::kPipelineStage_Kind && !function.fDefined && !function.fBuiltin) {
String msg = "call to undefined function '" + function.fName + "'";
fErrors.error(offset, msg);
return nullptr;
@@ -2314,11 +1866,6 @@
VariableReference::kPointer_RefKind);
}
}
- if (fCanInline && function.fDefinition && function.fDefinition->canBeInlined() &&
- ((fSettings->fCaps && fSettings->fCaps->canInlineEarlyReturns()) ||
- !has_early_return(*function.fDefinition))) {
- return this->inlineCall(offset, *function.fDefinition, std::move(arguments));
- }
return std::unique_ptr<FunctionCall>(new FunctionCall(offset, *returnType, function,
std::move(arguments)));
}
diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h
index 8e856bf..2895a34 100644
--- a/src/sksl/SkSLIRGenerator.h
+++ b/src/sksl/SkSLIRGenerator.h
@@ -83,22 +83,11 @@
std::unique_ptr<VarDeclarations> convertVarDeclarations(const ASTNode& decl,
Variable::Storage storage);
void convertFunction(const ASTNode& f);
- std::unique_ptr<Statement> convertSingleStatement(const ASTNode& statement);
std::unique_ptr<Statement> convertStatement(const ASTNode& statement);
std::unique_ptr<Expression> convertExpression(const ASTNode& expression);
std::unique_ptr<ModifiersDeclaration> convertModifiersDeclaration(const ASTNode& m);
const Type* convertType(const ASTNode& type);
- std::unique_ptr<Expression> inlineExpression(int offset,
- std::map<const Variable*, const Variable*>* varMap,
- const Expression& expression);
- std::unique_ptr<Statement> inlineStatement(int offset,
- std::map<const Variable*, const Variable*>* varMap,
- const Variable* returnVar,
- bool haveEarlyReturns,
- const Statement& statement);
- std::unique_ptr<Expression> inlineCall(int offset, const FunctionDefinition& function,
- std::vector<std::unique_ptr<Expression>> arguments);
std::unique_ptr<Expression> call(int offset,
const FunctionDeclaration& function,
std::vector<std::unique_ptr<Expression>> arguments);
@@ -167,9 +156,6 @@
std::unordered_map<String, Program::Settings::Value> fCapsMap;
std::shared_ptr<SymbolTable> fRootSymbolTable;
std::shared_ptr<SymbolTable> fSymbolTable;
- // additional statements that need to be inserted before the one that convertStatement is
- // currently working on
- std::vector<std::unique_ptr<Statement>> fExtraStatements;
// Symbols which have definitions in the include files. The bool tells us whether this
// intrinsic has been included already.
std::map<String, std::pair<std::unique_ptr<ProgramElement>, bool>>* fIntrinsics = nullptr;
@@ -182,7 +168,6 @@
Variable* fRTAdjust;
Variable* fRTAdjustInterfaceBlock;
int fRTAdjustFieldIndex;
- bool fCanInline = true;
friend class AutoSymbolTable;
friend class AutoLoopLevel;
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
index 2b222d8..099ab4c 100644
--- a/src/sksl/SkSLMetalCodeGenerator.cpp
+++ b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -1164,15 +1164,11 @@
}
void MetalCodeGenerator::writeBlock(const Block& b) {
- if (b.fIsScope) {
- this->writeLine("{");
- fIndentation++;
- }
+ this->writeLine("{");
+ fIndentation++;
this->writeStatements(b.fStatements);
- if (b.fIsScope) {
- fIndentation--;
- this->write("}");
- }
+ fIndentation--;
+ this->write("}");
}
void MetalCodeGenerator::writeIfStatement(const IfStatement& stmt) {
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index 6726a8f..b01bd95 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2987,6 +2987,14 @@
}
void SPIRVCodeGenerator::writeDoStatement(const DoStatement& d, OutputStream& out) {
+ // We believe the do loop code below will work, but Skia doesn't actually use them and
+ // adequately testing this code in the absence of Skia exercising it isn't straightforward. For
+ // the time being, we just fail with an error due to the lack of testing. If you encounter this
+ // message, simply remove the error call below to see whether our do loop support actually
+ // works.
+ fErrors.error(d.fOffset, "internal error: do loop support has been disabled in SPIR-V, see "
+ "SkSLSPIRVCodeGenerator.cpp for details");
+
SpvId header = this->nextId();
SpvId start = this->nextId();
SpvId next = this->nextId();
diff --git a/src/sksl/SkSLSectionAndParameterHelper.cpp b/src/sksl/SkSLSectionAndParameterHelper.cpp
index 6d99548..a897386 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.cpp
+++ b/src/sksl/SkSLSectionAndParameterHelper.cpp
@@ -199,9 +199,9 @@
}
case Statement::kFor_Kind: {
const ForStatement& f = (const ForStatement&) s;
- return (f.fInitializer ? this->hasCoordOverrides(*f.fInitializer, fp) : 0) ||
- (f.fTest ? this->hasCoordOverrides(*f.fTest, fp) : 0) ||
- (f.fNext ? this->hasCoordOverrides(*f.fNext, fp) : 0) ||
+ return this->hasCoordOverrides(*f.fInitializer, fp) ||
+ this->hasCoordOverrides(*f.fTest, fp) ||
+ this->hasCoordOverrides(*f.fNext, fp) ||
this->hasCoordOverrides(*f.fStatement, fp);
}
case Statement::kWhile_Kind: {
@@ -228,6 +228,7 @@
case Statement::kBreak_Kind:
case Statement::kContinue_Kind:
case Statement::kDiscard_Kind:
+ case Statement::kGroup_Kind:
case Statement::kNop_Kind:
return false;
}
@@ -320,13 +321,6 @@
return SampleMatrix();
}
-SampleMatrix SectionAndParameterHelper::getMatrix(const Expression* e, const Variable& fp) {
- if (e) {
- return this->getMatrix(*e, fp);
- }
- return SampleMatrix();
-}
-
SampleMatrix SectionAndParameterHelper::getMatrix(const Statement& s, const Variable& fp) {
switch (s.fKind) {
case Statement::kBlock_Kind: {
@@ -367,9 +361,9 @@
}
case Statement::kFor_Kind: {
const ForStatement& f = (const ForStatement&) s;
- return this->getMatrix(f.fInitializer.get(), fp).merge(
- this->getMatrix(f.fTest.get(), fp).merge(
- this->getMatrix(f.fNext.get(), fp).merge(
+ return this->getMatrix(*f.fInitializer, fp).merge(
+ this->getMatrix(*f.fTest, fp).merge(
+ this->getMatrix(*f.fNext, fp).merge(
this->getMatrix(*f.fStatement, fp))));
}
case Statement::kWhile_Kind: {
@@ -393,6 +387,7 @@
case Statement::kBreak_Kind:
case Statement::kContinue_Kind:
case Statement::kDiscard_Kind:
+ case Statement::kGroup_Kind:
case Statement::kNop_Kind:
return SampleMatrix();
}
@@ -400,12 +395,4 @@
return SampleMatrix();
}
-SampleMatrix SectionAndParameterHelper::getMatrix(const Statement* s, const Variable& fp) {
- if (s) {
- return this->getMatrix(*s, fp);
- }
- return SampleMatrix();
-}
-
-
}
diff --git a/src/sksl/SkSLSectionAndParameterHelper.h b/src/sksl/SkSLSectionAndParameterHelper.h
index 4bc7a47..d52302a 100644
--- a/src/sksl/SkSLSectionAndParameterHelper.h
+++ b/src/sksl/SkSLSectionAndParameterHelper.h
@@ -122,10 +122,6 @@
SampleMatrix getMatrix(const Expression& e, const Variable& fp);
- SampleMatrix getMatrix(const Statement* s, const Variable& fp);
-
- SampleMatrix getMatrix(const Expression* e, const Variable& fp);
-
SampleMatrix getMatrix(const ProgramElement& p, const Variable& fp);
const Program& fProgram;
diff --git a/src/sksl/SkSLUtil.h b/src/sksl/SkSLUtil.h
index 89269db..08f2842 100644
--- a/src/sksl/SkSLUtil.h
+++ b/src/sksl/SkSLUtil.h
@@ -144,10 +144,6 @@
return true;
}
- bool canInlineEarlyReturns() const {
- return false;
- }
-
const char* shaderDerivativeExtensionString() const {
return nullptr;
}
diff --git a/src/sksl/ir/SkSLBinaryExpression.h b/src/sksl/ir/SkSLBinaryExpression.h
index f4a48e9..9fc8683 100644
--- a/src/sksl/ir/SkSLBinaryExpression.h
+++ b/src/sksl/ir/SkSLBinaryExpression.h
@@ -45,10 +45,6 @@
return fLeft->hasProperty(property) || fRight->hasProperty(property);
}
- int nodeCount() const override {
- return 1 + fLeft->nodeCount() + fRight->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new BinaryExpression(fOffset, fLeft->clone(), fOperator,
fRight->clone(), fType));
diff --git a/src/sksl/ir/SkSLBlock.h b/src/sksl/ir/SkSLBlock.h
index b9fdf29..8a4449a 100644
--- a/src/sksl/ir/SkSLBlock.h
+++ b/src/sksl/ir/SkSLBlock.h
@@ -18,11 +18,10 @@
*/
struct Block : public Statement {
Block(int offset, std::vector<std::unique_ptr<Statement>> statements,
- const std::shared_ptr<SymbolTable> symbols = nullptr, bool isScope = true)
+ const std::shared_ptr<SymbolTable> symbols = nullptr)
: INHERITED(offset, kBlock_Kind)
, fSymbols(std::move(symbols))
- , fStatements(std::move(statements))
- , fIsScope(isScope) {}
+ , fStatements(std::move(statements)) {}
bool isEmpty() const override {
for (const auto& s : fStatements) {
@@ -33,21 +32,12 @@
return true;
}
- int nodeCount() const override {
- int result = 1;
- for (const auto& s : fStatements) {
- result += s->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Statement> clone() const override {
std::vector<std::unique_ptr<Statement>> cloned;
for (const auto& s : fStatements) {
cloned.push_back(s->clone());
}
- return std::unique_ptr<Statement>(new Block(fOffset, std::move(cloned), fSymbols,
- fIsScope));
+ return std::unique_ptr<Statement>(new Block(fOffset, std::move(cloned), fSymbols));
}
String description() const override {
@@ -64,10 +54,6 @@
// because destroying statements can modify reference counts in symbols
const std::shared_ptr<SymbolTable> fSymbols;
std::vector<std::unique_ptr<Statement>> fStatements;
- // if isScope is false, this is just a group of statements rather than an actual language-level
- // block. This allows us to pass around multiple statements as if they were a single unit, with
- // no semantic impact.
- bool fIsScope;
typedef Statement INHERITED;
};
diff --git a/src/sksl/ir/SkSLBoolLiteral.h b/src/sksl/ir/SkSLBoolLiteral.h
index 85dd8c5..74f48ad 100644
--- a/src/sksl/ir/SkSLBoolLiteral.h
+++ b/src/sksl/ir/SkSLBoolLiteral.h
@@ -38,10 +38,6 @@
return fValue == b.fValue;
}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new BoolLiteral(fOffset, fValue, &fType));
}
diff --git a/src/sksl/ir/SkSLBreakStatement.h b/src/sksl/ir/SkSLBreakStatement.h
index daedece..ae0c198 100644
--- a/src/sksl/ir/SkSLBreakStatement.h
+++ b/src/sksl/ir/SkSLBreakStatement.h
@@ -20,10 +20,6 @@
BreakStatement(int offset)
: INHERITED(offset, kBreak_Kind) {}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new BreakStatement(fOffset));
}
diff --git a/src/sksl/ir/SkSLConstructor.h b/src/sksl/ir/SkSLConstructor.h
index cdd6f84..524a6ad 100644
--- a/src/sksl/ir/SkSLConstructor.h
+++ b/src/sksl/ir/SkSLConstructor.h
@@ -59,14 +59,6 @@
return false;
}
- int nodeCount() const override {
- int result = 1;
- for (const auto& a : fArguments) {
- result += a->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Expression> clone() const override {
std::vector<std::unique_ptr<Expression>> cloned;
for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLContinueStatement.h b/src/sksl/ir/SkSLContinueStatement.h
index 1e01ac8..ecd9c3f 100644
--- a/src/sksl/ir/SkSLContinueStatement.h
+++ b/src/sksl/ir/SkSLContinueStatement.h
@@ -20,10 +20,6 @@
ContinueStatement(int offset)
: INHERITED(offset, kContinue_Kind) {}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new ContinueStatement(fOffset));
}
diff --git a/src/sksl/ir/SkSLDiscardStatement.h b/src/sksl/ir/SkSLDiscardStatement.h
index 8cc7e3f..40f625c 100644
--- a/src/sksl/ir/SkSLDiscardStatement.h
+++ b/src/sksl/ir/SkSLDiscardStatement.h
@@ -20,10 +20,6 @@
DiscardStatement(int offset)
: INHERITED(offset, kDiscard_Kind) {}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new DiscardStatement(fOffset));
}
diff --git a/src/sksl/ir/SkSLDoStatement.h b/src/sksl/ir/SkSLDoStatement.h
index 474aac7..5cab5c8 100644
--- a/src/sksl/ir/SkSLDoStatement.h
+++ b/src/sksl/ir/SkSLDoStatement.h
@@ -23,10 +23,6 @@
, fStatement(std::move(statement))
, fTest(std::move(test)) {}
- int nodeCount() const override {
- return 1 + fStatement->nodeCount() + fTest->nodeCount();
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new DoStatement(fOffset, fStatement->clone(),
fTest->clone()));
diff --git a/src/sksl/ir/SkSLExpressionStatement.h b/src/sksl/ir/SkSLExpressionStatement.h
index 1732bbb..80a8d31 100644
--- a/src/sksl/ir/SkSLExpressionStatement.h
+++ b/src/sksl/ir/SkSLExpressionStatement.h
@@ -21,10 +21,6 @@
: INHERITED(expression->fOffset, kExpression_Kind)
, fExpression(std::move(expression)) {}
- int nodeCount() const override {
- return 1 + fExpression->nodeCount();
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new ExpressionStatement(fExpression->clone()));
}
diff --git a/src/sksl/ir/SkSLExternalFunctionCall.h b/src/sksl/ir/SkSLExternalFunctionCall.h
index bf70561..deb1f4d 100644
--- a/src/sksl/ir/SkSLExternalFunctionCall.h
+++ b/src/sksl/ir/SkSLExternalFunctionCall.h
@@ -36,14 +36,6 @@
return false;
}
- int nodeCount() const override {
- int result = 1;
- for (const auto& a : fArguments) {
- result += a->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Expression> clone() const override {
std::vector<std::unique_ptr<Expression>> cloned;
for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLExternalValueReference.h b/src/sksl/ir/SkSLExternalValueReference.h
index 868c6c9..aa0ec18 100644
--- a/src/sksl/ir/SkSLExternalValueReference.h
+++ b/src/sksl/ir/SkSLExternalValueReference.h
@@ -25,10 +25,6 @@
return property == Property::kSideEffects;
}
- int nodeCount() const override {
- return 1;
- }
-
String description() const override {
return String(fValue->fName);
}
diff --git a/src/sksl/ir/SkSLFieldAccess.h b/src/sksl/ir/SkSLFieldAccess.h
index 6cba566..59f4c16 100644
--- a/src/sksl/ir/SkSLFieldAccess.h
+++ b/src/sksl/ir/SkSLFieldAccess.h
@@ -35,10 +35,6 @@
return fBase->hasProperty(property);
}
- int nodeCount() const override {
- return 1 + fBase->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new FieldAccess(fBase->clone(), fFieldIndex,
fOwnerKind));
diff --git a/src/sksl/ir/SkSLFloatLiteral.h b/src/sksl/ir/SkSLFloatLiteral.h
index 9d81771..79f5dcf 100644
--- a/src/sksl/ir/SkSLFloatLiteral.h
+++ b/src/sksl/ir/SkSLFloatLiteral.h
@@ -53,10 +53,6 @@
return fValue;
}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new FloatLiteral(fOffset, fValue, &fType));
}
diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h
index 29c61bf..4906e19 100644
--- a/src/sksl/ir/SkSLForStatement.h
+++ b/src/sksl/ir/SkSLForStatement.h
@@ -28,36 +28,16 @@
, fNext(std::move(next))
, fStatement(std::move(statement)) {}
- int nodeCount() const override {
- int result = 1;
- if (fInitializer) {
- result += fInitializer->nodeCount();
- }
- if (fTest) {
- result += fTest->nodeCount();
- }
- if (fNext) {
- result += fNext->nodeCount();
- }
- result += fStatement->nodeCount();
- return result;
- }
-
std::unique_ptr<Statement> clone() const override {
- return std::unique_ptr<Statement>(new ForStatement(fOffset,
- fInitializer ? fInitializer->clone() : nullptr,
- fTest ? fTest->clone() : nullptr,
- fNext ? fNext->clone() : nullptr,
- fStatement->clone(),
- fSymbols));
+ return std::unique_ptr<Statement>(new ForStatement(fOffset, fInitializer->clone(),
+ fTest->clone(), fNext->clone(),
+ fStatement->clone(), fSymbols));
}
String description() const override {
String result("for (");
if (fInitializer) {
result += fInitializer->description();
- } else {
- result += ";";
}
result += " ";
if (fTest) {
diff --git a/src/sksl/ir/SkSLFunctionCall.h b/src/sksl/ir/SkSLFunctionCall.h
index f575879..296aa0b 100644
--- a/src/sksl/ir/SkSLFunctionCall.h
+++ b/src/sksl/ir/SkSLFunctionCall.h
@@ -36,14 +36,6 @@
return false;
}
- int nodeCount() const override {
- int result = 1;
- for (const auto& a : fArguments) {
- result += a->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Expression> clone() const override {
std::vector<std::unique_ptr<Expression>> cloned;
for (const auto& arg : fArguments) {
diff --git a/src/sksl/ir/SkSLFunctionDeclaration.h b/src/sksl/ir/SkSLFunctionDeclaration.h
index 0301ed7..f2a3e0f 100644
--- a/src/sksl/ir/SkSLFunctionDeclaration.h
+++ b/src/sksl/ir/SkSLFunctionDeclaration.h
@@ -17,8 +17,6 @@
namespace SkSL {
-struct FunctionDefinition;
-
/**
* A function declaration (not a definition -- does not contain a body).
*/
@@ -26,7 +24,7 @@
FunctionDeclaration(int offset, Modifiers modifiers, StringFragment name,
std::vector<const Variable*> parameters, const Type& returnType)
: INHERITED(offset, kFunctionDeclaration_Kind, std::move(name))
- , fDefinition(nullptr)
+ , fDefined(false)
, fBuiltin(false)
, fModifiers(modifiers)
, fParameters(std::move(parameters))
@@ -109,7 +107,7 @@
return true;
}
- mutable FunctionDefinition* fDefinition;
+ mutable bool fDefined;
bool fBuiltin;
Modifiers fModifiers;
const std::vector<const Variable*> fParameters;
diff --git a/src/sksl/ir/SkSLFunctionDefinition.h b/src/sksl/ir/SkSLFunctionDefinition.h
index 4bcadcf..511a0f8 100644
--- a/src/sksl/ir/SkSLFunctionDefinition.h
+++ b/src/sksl/ir/SkSLFunctionDefinition.h
@@ -26,11 +26,6 @@
, fDeclaration(declaration)
, fBody(std::move(body)) {}
- bool canBeInlined() const {
- static const int INLINE_THRESHOLD = 50; // chosen arbitrarily, feel free to adjust
- return fBody->nodeCount() < INLINE_THRESHOLD;
- }
-
std::unique_ptr<ProgramElement> clone() const override {
return std::unique_ptr<ProgramElement>(new FunctionDefinition(fOffset, fDeclaration,
fBody->clone()));
diff --git a/src/sksl/ir/SkSLIRNode.h b/src/sksl/ir/SkSLIRNode.h
index cace825..ca9ea99 100644
--- a/src/sksl/ir/SkSLIRNode.h
+++ b/src/sksl/ir/SkSLIRNode.h
@@ -23,12 +23,6 @@
virtual ~IRNode() {}
- virtual int nodeCount() const {
- SkASSERT(false);
- return 1;
- }
-
-
virtual String description() const = 0;
// character offset of this element within the program being compiled, for error reporting
diff --git a/src/sksl/ir/SkSLIfStatement.h b/src/sksl/ir/SkSLIfStatement.h
index 6af6740..5d0a22b 100644
--- a/src/sksl/ir/SkSLIfStatement.h
+++ b/src/sksl/ir/SkSLIfStatement.h
@@ -25,11 +25,6 @@
, fIfTrue(std::move(ifTrue))
, fIfFalse(std::move(ifFalse)) {}
- int nodeCount() const override {
- return 1 + fTest->nodeCount() + fIfTrue->nodeCount() +
- (fIfFalse ? fIfFalse->nodeCount() : 0);
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new IfStatement(fOffset, fIsStatic, fTest->clone(),
fIfTrue->clone(), fIfFalse ? fIfFalse->clone() : nullptr));
diff --git a/src/sksl/ir/SkSLIndexExpression.h b/src/sksl/ir/SkSLIndexExpression.h
index 2018881..2b3a3a9 100644
--- a/src/sksl/ir/SkSLIndexExpression.h
+++ b/src/sksl/ir/SkSLIndexExpression.h
@@ -62,10 +62,6 @@
return fBase->hasProperty(property) || fIndex->hasProperty(property);
}
- int nodeCount() const override {
- return 1 + fBase->nodeCount() + fIndex->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new IndexExpression(fBase->clone(), fIndex->clone(),
&fType));
diff --git a/src/sksl/ir/SkSLIntLiteral.h b/src/sksl/ir/SkSLIntLiteral.h
index 4977fbf..6c686e1 100644
--- a/src/sksl/ir/SkSLIntLiteral.h
+++ b/src/sksl/ir/SkSLIntLiteral.h
@@ -56,10 +56,6 @@
return fValue;
}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new IntLiteral(fOffset, fValue, &fType));
}
diff --git a/src/sksl/ir/SkSLNop.h b/src/sksl/ir/SkSLNop.h
index 787060d..2ead371 100644
--- a/src/sksl/ir/SkSLNop.h
+++ b/src/sksl/ir/SkSLNop.h
@@ -28,10 +28,6 @@
return String(";");
}
- int nodeCount() const override {
- return 0;
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new Nop());
}
diff --git a/src/sksl/ir/SkSLNullLiteral.h b/src/sksl/ir/SkSLNullLiteral.h
index 0d04b77..d04fc5c 100644
--- a/src/sksl/ir/SkSLNullLiteral.h
+++ b/src/sksl/ir/SkSLNullLiteral.h
@@ -39,10 +39,6 @@
return true;
}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new NullLiteral(fOffset, fType));
}
diff --git a/src/sksl/ir/SkSLPostfixExpression.h b/src/sksl/ir/SkSLPostfixExpression.h
index c6f89eb..d95f687 100644
--- a/src/sksl/ir/SkSLPostfixExpression.h
+++ b/src/sksl/ir/SkSLPostfixExpression.h
@@ -30,10 +30,6 @@
return fOperand->hasProperty(property);
}
- int nodeCount() const override {
- return 1 + fOperand->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new PostfixExpression(fOperand->clone(), fOperator));
}
diff --git a/src/sksl/ir/SkSLPrefixExpression.h b/src/sksl/ir/SkSLPrefixExpression.h
index ab0363e..76f144b 100644
--- a/src/sksl/ir/SkSLPrefixExpression.h
+++ b/src/sksl/ir/SkSLPrefixExpression.h
@@ -64,10 +64,6 @@
return -fOperand->getMatComponent(col, row);
}
- int nodeCount() const override {
- return 1 + fOperand->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new PrefixExpression(fOperator, fOperand->clone()));
}
diff --git a/src/sksl/ir/SkSLReturnStatement.h b/src/sksl/ir/SkSLReturnStatement.h
index 82b4678..e61fa36 100644
--- a/src/sksl/ir/SkSLReturnStatement.h
+++ b/src/sksl/ir/SkSLReturnStatement.h
@@ -24,10 +24,6 @@
: INHERITED(expression->fOffset, kReturn_Kind)
, fExpression(std::move(expression)) {}
- int nodeCount() const override {
- return 1 + fExpression->nodeCount();
- }
-
std::unique_ptr<Statement> clone() const override {
if (fExpression) {
return std::unique_ptr<Statement>(new ReturnStatement(fExpression->clone()));
diff --git a/src/sksl/ir/SkSLSetting.h b/src/sksl/ir/SkSLSetting.h
index 9a69802..a47b42b 100644
--- a/src/sksl/ir/SkSLSetting.h
+++ b/src/sksl/ir/SkSLSetting.h
@@ -28,10 +28,6 @@
std::unique_ptr<Expression> constantPropagate(const IRGenerator& irGenerator,
const DefinitionMap& definitions) override;
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new Setting(fOffset, fName, fValue->clone()));
}
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
index 2260cdd..ed8d33b 100644
--- a/src/sksl/ir/SkSLStatement.h
+++ b/src/sksl/ir/SkSLStatement.h
@@ -25,6 +25,7 @@
kDo_Kind,
kExpression_Kind,
kFor_Kind,
+ kGroup_Kind,
kIf_Kind,
kNop_Kind,
kReturn_Kind,
diff --git a/src/sksl/ir/SkSLSwitchCase.h b/src/sksl/ir/SkSLSwitchCase.h
index dcfe956..b1ddb01 100644
--- a/src/sksl/ir/SkSLSwitchCase.h
+++ b/src/sksl/ir/SkSLSwitchCase.h
@@ -23,17 +23,6 @@
, fValue(std::move(value))
, fStatements(std::move(statements)) {}
- int nodeCount() const override {
- int result = 1;
- if (fValue) {
- result += fValue->nodeCount();
- }
- for (const auto& s : fStatements) {
- result += s->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Statement> clone() const override {
std::vector<std::unique_ptr<Statement>> cloned;
for (const auto& s : fStatements) {
diff --git a/src/sksl/ir/SkSLSwitchStatement.h b/src/sksl/ir/SkSLSwitchStatement.h
index a85420a..0777c5c 100644
--- a/src/sksl/ir/SkSLSwitchStatement.h
+++ b/src/sksl/ir/SkSLSwitchStatement.h
@@ -28,14 +28,6 @@
, fSymbols(std::move(symbols))
, fCases(std::move(cases)) {}
- int nodeCount() const override {
- int result = 1 + fValue->nodeCount();
- for (const auto& c : fCases) {
- result += c->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Statement> clone() const override {
std::vector<std::unique_ptr<SwitchCase>> cloned;
for (const auto& s : fCases) {
diff --git a/src/sksl/ir/SkSLSwizzle.h b/src/sksl/ir/SkSLSwizzle.h
index 59ff04c..3428f28 100644
--- a/src/sksl/ir/SkSLSwizzle.h
+++ b/src/sksl/ir/SkSLSwizzle.h
@@ -136,10 +136,6 @@
return fBase->hasProperty(property);
}
- int nodeCount() const override {
- return 1 + fBase->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new Swizzle(fType, fBase->clone(), fComponents));
}
diff --git a/src/sksl/ir/SkSLSymbol.h b/src/sksl/ir/SkSLSymbol.h
index df606b8..4ee4d3d 100644
--- a/src/sksl/ir/SkSLSymbol.h
+++ b/src/sksl/ir/SkSLSymbol.h
@@ -30,7 +30,7 @@
, fKind(kind)
, fName(name) {}
- virtual ~Symbol() override {}
+ virtual ~Symbol() {}
Kind fKind;
StringFragment fName;
diff --git a/src/sksl/ir/SkSLSymbolTable.cpp b/src/sksl/ir/SkSLSymbolTable.cpp
index b0a2a56..bbf001d 100644
--- a/src/sksl/ir/SkSLSymbolTable.cpp
+++ b/src/sksl/ir/SkSLSymbolTable.cpp
@@ -72,12 +72,6 @@
return result;
}
-String* SymbolTable::takeOwnership(std::unique_ptr<String> n) {
- String* result = n.get();
- fOwnedStrings.push_back(std::move(n));
- return result;
-}
-
void SymbolTable::add(StringFragment name, std::unique_ptr<Symbol> symbol) {
this->addWithoutOwnership(name, symbol.get());
this->takeOwnership(std::move(symbol));
diff --git a/src/sksl/ir/SkSLSymbolTable.h b/src/sksl/ir/SkSLSymbolTable.h
index a3f89e2..6969ba5 100644
--- a/src/sksl/ir/SkSLSymbolTable.h
+++ b/src/sksl/ir/SkSLSymbolTable.h
@@ -41,8 +41,6 @@
IRNode* takeOwnership(std::unique_ptr<IRNode> n);
- String* takeOwnership(std::unique_ptr<String> n);
-
void markAllFunctionsBuiltin();
std::unordered_map<StringFragment, const Symbol*>::iterator begin();
@@ -58,8 +56,6 @@
std::vector<std::unique_ptr<IRNode>> fOwnedNodes;
- std::vector<std::unique_ptr<String>> fOwnedStrings;
-
std::unordered_map<StringFragment, const Symbol*> fSymbols;
ErrorReporter& fErrorReporter;
diff --git a/src/sksl/ir/SkSLTernaryExpression.h b/src/sksl/ir/SkSLTernaryExpression.h
index cb4c004..80dcb8f 100644
--- a/src/sksl/ir/SkSLTernaryExpression.h
+++ b/src/sksl/ir/SkSLTernaryExpression.h
@@ -36,10 +36,6 @@
fIfFalse->isConstantOrUniform();
}
- int nodeCount() const override {
- return 1 + fTest->nodeCount() + fIfTrue->nodeCount() + fIfFalse->nodeCount();
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new TernaryExpression(fOffset, fTest->clone(),
fIfTrue->clone(),
diff --git a/src/sksl/ir/SkSLVarDeclarations.h b/src/sksl/ir/SkSLVarDeclarations.h
index 86aafdb..82dbd86 100644
--- a/src/sksl/ir/SkSLVarDeclarations.h
+++ b/src/sksl/ir/SkSLVarDeclarations.h
@@ -29,19 +29,6 @@
, fSizes(std::move(sizes))
, fValue(std::move(value)) {}
- int nodeCount() const override {
- int result = 1;
- for (const auto& s : fSizes) {
- if (s) {
- result += s->nodeCount();
- }
- }
- if (fValue) {
- result += fValue->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<Statement> clone() const override {
std::vector<std::unique_ptr<Expression>> sizesClone;
for (const auto& s : fSizes) {
@@ -90,14 +77,6 @@
}
}
- int nodeCount() const override {
- int result = 1;
- for (const auto& v : fVars) {
- result += v->nodeCount();
- }
- return result;
- }
-
std::unique_ptr<ProgramElement> clone() const override {
std::vector<std::unique_ptr<VarDeclaration>> cloned;
for (const auto& v : fVars) {
@@ -112,15 +91,8 @@
if (!fVars.size()) {
return String();
}
- String result;
- for (const auto& var : fVars) {
- if (var->fKind != Statement::kNop_Kind) {
- SkASSERT(var->fKind == Statement::kVarDeclaration_Kind);
- result = ((const VarDeclaration&) *var).fVar->fModifiers.description();
- break;
- }
- }
- result += fBaseType.description() + " ";
+ String result = ((VarDeclaration&) *fVars[0]).fVar->fModifiers.description() +
+ fBaseType.description() + " ";
String separator;
for (const auto& var : fVars) {
result += separator;
diff --git a/src/sksl/ir/SkSLVarDeclarationsStatement.h b/src/sksl/ir/SkSLVarDeclarationsStatement.h
index eb7880f..465b691 100644
--- a/src/sksl/ir/SkSLVarDeclarationsStatement.h
+++ b/src/sksl/ir/SkSLVarDeclarationsStatement.h
@@ -30,10 +30,6 @@
return true;
}
- int nodeCount() const override {
- return 1 + fDeclaration->nodeCount();
- }
-
std::unique_ptr<Statement> clone() const override {
std::unique_ptr<VarDeclarations> cloned((VarDeclarations*) fDeclaration->clone().release());
return std::unique_ptr<Statement>(new VarDeclarationsStatement(std::move(cloned)));
diff --git a/src/sksl/ir/SkSLVariableReference.h b/src/sksl/ir/SkSLVariableReference.h
index c675144..5d962b9 100644
--- a/src/sksl/ir/SkSLVariableReference.h
+++ b/src/sksl/ir/SkSLVariableReference.h
@@ -62,10 +62,6 @@
return (fVariable.fModifiers.fFlags & Modifiers::kUniform_Flag) != 0;
}
- int nodeCount() const override {
- return 1;
- }
-
std::unique_ptr<Expression> clone() const override {
return std::unique_ptr<Expression>(new VariableReference(fOffset, fVariable, fRefKind));
}
diff --git a/src/sksl/ir/SkSLWhileStatement.h b/src/sksl/ir/SkSLWhileStatement.h
index 0782a01..8e311a0 100644
--- a/src/sksl/ir/SkSLWhileStatement.h
+++ b/src/sksl/ir/SkSLWhileStatement.h
@@ -23,10 +23,6 @@
, fTest(std::move(test))
, fStatement(std::move(statement)) {}
- int nodeCount() const override {
- return 1 + fTest->nodeCount() + fStatement->nodeCount();
- }
-
std::unique_ptr<Statement> clone() const override {
return std::unique_ptr<Statement>(new WhileStatement(fOffset, fTest->clone(),
fStatement->clone()));
diff --git a/src/sksl/sksl_blend.inc b/src/sksl/sksl_blend.inc
index ab6dcb8..8fc3415 100644
--- a/src/sksl/sksl_blend.inc
+++ b/src/sksl/sksl_blend.inc
@@ -69,9 +69,8 @@
half _guarded_divide(half n, half d) {
@if (sk_Caps.mustGuardDivisionEvenAfterExplicitZeroCheck) {
return n/(d + 0.00000001);
- } else {
- return n/d;
}
+ return n/d;
}
half _color_dodge_component(half sc, half sa, half dc, half da) {
diff --git a/tests/SkSLErrorTest.cpp b/tests/SkSLErrorTest.cpp
index 73e364a..53214d2 100644
--- a/tests/SkSLErrorTest.cpp
+++ b/tests/SkSLErrorTest.cpp
@@ -371,7 +371,7 @@
"void main() { if (true) return; else discard; return; }",
"error: 1: unreachable\n1 error\n");
test_failure(r,
- "void main() { return; main(); }",
+ "void main() { return; while (true); }",
"error: 1: unreachable\n1 error\n");
}
diff --git a/tests/SkSLFPTest.cpp b/tests/SkSLFPTest.cpp
index 23c4fd7..c1de765 100644
--- a/tests/SkSLFPTest.cpp
+++ b/tests/SkSLFPTest.cpp
@@ -784,8 +784,8 @@
"const GrShaderVar flip_args[] = { GrShaderVar(\"c\", kHalf4_GrSLType)};",
"fragBuilder->emitFunction(kHalf4_GrSLType, \"flip\", 1, flip_args, "
"\"return c.wzyx;\\n\", &flip_name);",
- "fragBuilder->codeAppendf(\"half4 inlineResult101;\\nhalf4 inlineArg101_0 = %s;\\n{\\n inlineResult101 = inlineArg101_0.wzyx;\\n}\\n%s = inlineResult101;\\n\\n\", args.fInputColor, "
- "args.fOutputColor);"
+ "fragBuilder->codeAppendf(\"%s = %s(%s);\\n\", args.fOutputColor, flip_name.c_str(), "
+ "args.fInputColor);"
});
}
diff --git a/tests/SkSLGLSLTest.cpp b/tests/SkSLGLSLTest.cpp
index faddfbe..a67ca25 100644
--- a/tests/SkSLGLSLTest.cpp
+++ b/tests/SkSLGLSLTest.cpp
@@ -111,33 +111,12 @@
" float y[2], z;\n"
" y[0] = x;\n"
" y[1] = x * 2.0;\n"
- " float inlineResult117;\n"
- " float[2] inlineArg117_0 = y;\n"
- " {\n"
- " inlineResult117 = inlineArg117_0[0] * inlineArg117_0[1];\n"
- " }\n"
- " z = inlineResult117;\n"
- "\n"
+ " z = foo(y);\n"
" x = z;\n"
"}\n"
"void main() {\n"
" float x = 10.0;\n"
- " float inlineArg161_0 = x;\n"
- " {\n"
- " float y[2], z;\n"
- " y[0] = inlineArg161_0;\n"
- " y[1] = inlineArg161_0 * 2.0;\n"
- " float inlineResult117;\n"
- " float[2] inlineArg117_0 = y;\n"
- " {\n"
- " inlineResult117 = inlineArg117_0[0] * inlineArg117_0[1];\n"
- " }\n"
- " z = inlineResult117;\n"
- "\n"
- " inlineArg161_0 = z;\n"
- " }\n"
- " x = inlineArg161_0;\n"
- "\n"
+ " bar(x);\n"
" sk_FragColor = vec4(x);\n"
"}\n");
}