From b321a07f16dc6bace0d93676bf943964ca41d653 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Thu, 25 Feb 2021 16:24:19 -0500 Subject: [PATCH] Convert IRGenerator::convertFor to ForStatement::Make. Since while statements are implemented in terms of a for loop, also added ForStatement::MakeWhile() which assumes null for the init-stmt and the next-expr. We currently don't have any optimizations for for-statements so the primary benefit is moving code out of IRGenerator. Change-Id: I4b3fc3482e28b7d28065e85670a6037b511847ff Bug: skia:11342 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/375203 Reviewed-by: Ethan Nicholas Commit-Queue: Ethan Nicholas Auto-Submit: John Stiles --- gn/sksl.gni | 1 + src/sksl/SkSLIRGenerator.cpp | 61 ++++-------------------- src/sksl/SkSLIRGenerator.h | 7 --- src/sksl/SkSLInliner.cpp | 18 +++---- src/sksl/dsl/DSLCore.cpp | 8 ++-- src/sksl/ir/SkSLForStatement.cpp | 82 ++++++++++++++++++++++++++++++++ src/sksl/ir/SkSLForStatement.h | 43 +++++++---------- 7 files changed, 123 insertions(+), 97 deletions(-) create mode 100644 src/sksl/ir/SkSLForStatement.cpp diff --git a/gn/sksl.gni b/gn/sksl.gni index 4e947365c6..75766ebc55 100644 --- a/gn/sksl.gni +++ b/gn/sksl.gni @@ -100,6 +100,7 @@ skia_sksl_sources = [ "$_src/sksl/ir/SkSLField.h", "$_src/sksl/ir/SkSLFieldAccess.h", "$_src/sksl/ir/SkSLFloatLiteral.h", + "$_src/sksl/ir/SkSLForStatement.cpp", "$_src/sksl/ir/SkSLForStatement.h", "$_src/sksl/ir/SkSLFunctionCall.h", "$_src/sksl/ir/SkSLFunctionDeclaration.h", diff --git a/src/sksl/SkSLIRGenerator.cpp b/src/sksl/SkSLIRGenerator.cpp index 353a186933..5319c36ad7 100644 --- a/src/sksl/SkSLIRGenerator.cpp +++ b/src/sksl/SkSLIRGenerator.cpp @@ -579,30 +579,6 @@ std::unique_ptr IRGenerator::convertIf(const ASTNode& n) { std::move(ifTrue), std::move(ifFalse)); } -std::unique_ptr IRGenerator::convertFor(int offset, - std::unique_ptr initializer, - std::unique_ptr test, - std::unique_ptr next, - std::unique_ptr statement) { - if (test) { - test = this->coerce(std::move(test), *fContext.fTypes.fBool); - if (!test) { - return nullptr; - } - } - - auto forStmt = - std::make_unique(offset, std::move(initializer), std::move(test), - std::move(next), std::move(statement), fSymbolTable); - if (this->strictES2Mode()) { - if (!Analysis::ForLoopIsValidForES2(*forStmt, /*outLoopInfo=*/nullptr, - &this->errorReporter())) { - return nullptr; - } - } - return std::move(forStmt); -} - std::unique_ptr IRGenerator::convertFor(const ASTNode& f) { SkASSERT(f.fKind == ASTNode::Kind::kFor); AutoSymbolTable table(this); @@ -636,27 +612,8 @@ std::unique_ptr IRGenerator::convertFor(const ASTNode& f) { return nullptr; } - return this->convertFor(f.fOffset, std::move(initializer), std::move(test), std::move(next), - std::move(statement)); -} - -std::unique_ptr IRGenerator::convertWhile(int offset, std::unique_ptr test, - std::unique_ptr statement) { - if (this->strictES2Mode()) { - this->errorReporter().error(offset, "while loops are not supported"); - return nullptr; - } - - test = this->coerce(std::move(test), *fContext.fTypes.fBool); - if (!test) { - return nullptr; - } - if (this->detectVarDeclarationWithoutScope(*statement)) { - return nullptr; - } - - return std::make_unique(offset, /*initializer=*/nullptr, std::move(test), - /*next=*/nullptr, std::move(statement), fSymbolTable); + return ForStatement::Make(fContext, f.fOffset, std::move(initializer), std::move(test), + std::move(next), std::move(statement), fSymbolTable); } std::unique_ptr IRGenerator::convertWhile(const ASTNode& w) { @@ -670,7 +627,11 @@ std::unique_ptr IRGenerator::convertWhile(const ASTNode& w) { if (!statement) { return nullptr; } - return this->convertWhile(w.fOffset, std::move(test), std::move(statement)); + if (this->detectVarDeclarationWithoutScope(*statement)) { + return nullptr; + } + return ForStatement::MakeWhile(fContext, w.fOffset, std::move(test), std::move(statement), + fSymbolTable); } std::unique_ptr IRGenerator::convertDo(std::unique_ptr stmt, @@ -838,11 +799,9 @@ std::unique_ptr IRGenerator::applyInvocationIDWorkaround(std::unique_ptr< std::make_unique(fContext, /*offset=*/-1, /*value=*/0), fContext.fTypes.fInt.get()); auto initializer = std::make_unique(std::move(assignment)); - auto loop = std::make_unique(/*offset=*/-1, - std::move(initializer), - std::move(test), std::move(next), - std::make_unique(-1, std::move(loopBody)), - fSymbolTable); + auto loop = ForStatement::Make( + fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next), + std::make_unique(/*offset=*/-1, std::move(loopBody)), fSymbolTable); StatementArray children; children.push_back(std::move(loop)); return std::make_unique(/*offset=*/-1, std::move(children)); diff --git a/src/sksl/SkSLIRGenerator.h b/src/sksl/SkSLIRGenerator.h index 7e98bb9a4e..8a183494d7 100644 --- a/src/sksl/SkSLIRGenerator.h +++ b/src/sksl/SkSLIRGenerator.h @@ -203,11 +203,6 @@ private: std::unique_ptr convertExpressionStatement(const ASTNode& s); std::unique_ptr convertField(std::unique_ptr base, StringFragment field); - std::unique_ptr convertFor(int offset, - std::unique_ptr initializer, - std::unique_ptr test, - std::unique_ptr next, - std::unique_ptr statement); std::unique_ptr convertFor(const ASTNode& f); std::unique_ptr convertIdentifier(int offset, StringFragment identifier); std::unique_ptr convertIdentifier(const ASTNode& identifier); @@ -238,8 +233,6 @@ private: std::unique_ptr ifFalse); std::unique_ptr convertTernaryExpression(const ASTNode& expression); std::unique_ptr convertVarDeclarationStatement(const ASTNode& s); - std::unique_ptr convertWhile(int offset, std::unique_ptr test, - std::unique_ptr statement); std::unique_ptr convertWhile(const ASTNode& w); void convertGlobalVarDeclarations(const ASTNode& decl); void convertEnum(const ASTNode& e); diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp index 59e696693c..1ec491b64c 100644 --- a/src/sksl/SkSLInliner.cpp +++ b/src/sksl/SkSLInliner.cpp @@ -461,9 +461,9 @@ std::unique_ptr Inliner::inlineStatement(int offset, // 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 initializer = stmt(f.initializer()); - return std::make_unique(offset, std::move(initializer), expr(f.test()), - expr(f.next()), stmt(f.statement()), - SymbolTable::WrapIfBuiltin(f.symbols())); + return ForStatement::Make(*fContext, offset, std::move(initializer), expr(f.test()), + expr(f.next()), stmt(f.statement()), + SymbolTable::WrapIfBuiltin(f.symbols())); } case Statement::Kind::kIf: { const IfStatement& i = statement.as(); @@ -726,12 +726,12 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call, inlineStatements = &innerBlock->children(); // for (int _1_loop = 0; _1_loop < 1; _1_loop++) {...} - inlinedBody.children().push_back(std::make_unique(/*offset=*/-1, - std::move(loopVar.fVarDecl), - std::move(test), - std::move(increment), - std::move(innerBlock), - symbolTable)); + inlinedBody.children().push_back(ForStatement::Make(*fContext, /*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 into our existing scopeless block. inlineStatements = &inlinedBody.children(); diff --git a/src/sksl/dsl/DSLCore.cpp b/src/sksl/dsl/DSLCore.cpp index 1200be1840..0f84b48019 100644 --- a/src/sksl/dsl/DSLCore.cpp +++ b/src/sksl/dsl/DSLCore.cpp @@ -93,8 +93,9 @@ public: static DSLStatement For(DSLStatement initializer, DSLExpression test, DSLExpression next, DSLStatement stmt) { - return DSLWriter::IRGenerator().convertFor(/*offset=*/-1, initializer.release(), - test.release(), next.release(), stmt.release()); + return ForStatement::Make(DSLWriter::Context(), /*offset=*/-1, initializer.release(), + test.release(), next.release(), stmt.release(), + DSLWriter::SymbolTable()); } static DSLStatement If(DSLExpression test, DSLStatement ifTrue, DSLStatement ifFalse) { @@ -146,7 +147,8 @@ public: } static DSLStatement While(DSLExpression test, DSLStatement stmt) { - return DSLWriter::IRGenerator().convertWhile(/*offset=*/-1, test.release(), stmt.release()); + return ForStatement::MakeWhile(DSLWriter::Context(), /*offset=*/-1, test.release(), + stmt.release(), DSLWriter::SymbolTable()); } }; diff --git a/src/sksl/ir/SkSLForStatement.cpp b/src/sksl/ir/SkSLForStatement.cpp new file mode 100644 index 0000000000..75dfeb4c85 --- /dev/null +++ b/src/sksl/ir/SkSLForStatement.cpp @@ -0,0 +1,82 @@ +/* + * Copyright 2021 Google LLC + * + * Use of this source code is governed by a BSD-style license that can be + * found in the LICENSE file. + */ + +#include "src/sksl/SkSLAnalysis.h" +#include "src/sksl/SkSLContext.h" +#include "src/sksl/SkSLProgramSettings.h" +#include "src/sksl/ir/SkSLForStatement.h" +#include "src/sksl/ir/SkSLSymbolTable.h" +#include "src/sksl/ir/SkSLType.h" + +namespace SkSL { + +std::unique_ptr ForStatement::clone() const { + return std::make_unique( + fOffset, + this->initializer() ? this->initializer()->clone() : nullptr, + this->test() ? this->test()->clone() : nullptr, + this->next() ? this->next()->clone() : nullptr, + this->statement()->clone(), + SymbolTable::WrapIfBuiltin(this->symbols())); +} + +String ForStatement::description() const { + String result("for ("); + if (this->initializer()) { + result += this->initializer()->description(); + } else { + result += ";"; + } + result += " "; + if (this->test()) { + result += this->test()->description(); + } + result += "; "; + if (this->next()) { + result += this->next()->description(); + } + result += ") " + this->statement()->description(); + return result; +} + +std::unique_ptr ForStatement::Make(const Context& context, int offset, + std::unique_ptr initializer, + std::unique_ptr test, + std::unique_ptr next, + std::unique_ptr statement, + std::shared_ptr symbolTable) { + if (test) { + test = context.fTypes.fBool->coerceExpression(std::move(test), context); + if (!test) { + return nullptr; + } + } + + auto forStmt = std::make_unique(offset, std::move(initializer), std::move(test), + std::move(next), std::move(statement), + std::move(symbolTable)); + if (context.fConfig->strictES2Mode()) { + if (!Analysis::ForLoopIsValidForES2(*forStmt, /*outLoopInfo=*/nullptr, &context.fErrors)) { + return nullptr; + } + } + return std::move(forStmt); +} + +std::unique_ptr ForStatement::MakeWhile(const Context& context, int offset, + std::unique_ptr test, + std::unique_ptr statement, + std::shared_ptr symbolTable) { + if (context.fConfig->strictES2Mode()) { + context.fErrors.error(offset, "while loops are not supported"); + return nullptr; + } + return Make(context, offset, /*initializer=*/nullptr, std::move(test), /*next=*/nullptr, + std::move(statement), std::move(symbolTable)); +} + +} // namespace SkSL diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h index 8aaafec450..59539f21a0 100644 --- a/src/sksl/ir/SkSLForStatement.h +++ b/src/sksl/ir/SkSLForStatement.h @@ -31,6 +31,20 @@ public: , fNext(std::move(next)) , fStatement(std::move(statement)) {} + // Creates an SkSL for loop. + static std::unique_ptr Make(const Context& context, int offset, + std::unique_ptr initializer, + std::unique_ptr test, + std::unique_ptr next, + std::unique_ptr statement, + std::shared_ptr symbolTable); + + // A while statement is represented in SkSL as a for loop with no init-stmt or next-expr. + static std::unique_ptr MakeWhile(const Context& context, int offset, + std::unique_ptr test, + std::unique_ptr statement, + std::shared_ptr symbolTable); + std::unique_ptr& initializer() { return fInitializer; } @@ -67,34 +81,9 @@ public: return fSymbolTable; } - std::unique_ptr clone() const override { - return std::make_unique( - fOffset, - this->initializer() ? this->initializer()->clone() : nullptr, - this->test() ? this->test()->clone() : nullptr, - this->next() ? this->next()->clone() : nullptr, - this->statement()->clone(), - SymbolTable::WrapIfBuiltin(this->symbols())); - } + std::unique_ptr clone() const override; - String description() const override { - String result("for ("); - if (this->initializer()) { - result += this->initializer()->description(); - } else { - result += ";"; - } - result += " "; - if (this->test()) { - result += this->test()->description(); - } - result += "; "; - if (this->next()) { - result += this->next()->description(); - } - result += ") " + this->statement()->description(); - return result; - } + String description() const override; private: std::shared_ptr fSymbolTable;