From 9c975c52d65701522794bbc151f74a375bba7787 Mon Sep 17 00:00:00 2001 From: John Stiles Date: Tue, 31 Aug 2021 10:18:57 -0400 Subject: [PATCH] Store loop-unroll information inside ForStatement. This will be useful when trying to determine the flattened size of a program, and it is expensive to compute on demand. Change-Id: I232d9189511502d4783e5542a9bfe0dff8ea8c4a Bug: skia:12396 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/443883 Commit-Queue: John Stiles Commit-Queue: Brian Osman Auto-Submit: John Stiles Reviewed-by: Brian Osman --- src/sksl/SkSLAnalysis.cpp | 26 +++++++-------- src/sksl/SkSLAnalysis.h | 26 ++++++--------- src/sksl/SkSLInliner.cpp | 4 ++- src/sksl/SkSLRehydrator.cpp | 2 +- src/sksl/codegen/SkSLVMCodeGenerator.cpp | 6 ++-- src/sksl/ir/SkSLForStatement.cpp | 32 +++++++++++++------ src/sksl/ir/SkSLForStatement.h | 40 ++++++++++++++++++------ 7 files changed, 81 insertions(+), 55 deletions(-) diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp index 52dbc84563d5..3a79c5740b21 100644 --- a/src/sksl/SkSLAnalysis.cpp +++ b/src/sksl/SkSLAnalysis.cpp @@ -937,7 +937,7 @@ static const char* invalid_for_ES2(int offset, const Expression* loopTest, const Expression* loopNext, const Statement* loopStatement, - Analysis::UnrollableLoopInfo& loopInfo) { + LoopUnrollInfo& loopInfo) { // // init_declaration has the form: type_specifier identifier = constant_expression // @@ -1093,23 +1093,21 @@ static const char* invalid_for_ES2(int offset, return nullptr; // All checks pass } -bool Analysis::ForLoopIsValidForES2(int offset, - const Statement* loopInitializer, - const Expression* loopTest, - const Expression* loopNext, - const Statement* loopStatement, - Analysis::UnrollableLoopInfo* outLoopInfo, - ErrorReporter* errors) { - UnrollableLoopInfo ignored, - *loopInfo = outLoopInfo ? outLoopInfo : &ignored; - if (const char* msg = invalid_for_ES2( - offset, loopInitializer, loopTest, loopNext, loopStatement, *loopInfo)) { +std::unique_ptr Analysis::GetLoopUnrollInfo(int offset, + const Statement* loopInitializer, + const Expression* loopTest, + const Expression* loopNext, + const Statement* loopStatement, + ErrorReporter* errors) { + auto result = std::make_unique(); + if (const char* msg = invalid_for_ES2(offset, loopInitializer, loopTest, loopNext, + loopStatement, *result)) { + result = nullptr; if (errors) { errors->error(offset, msg); } - return false; } - return true; + return result; } // Checks for ES2 constant-expression rules, and (optionally) constant-index-expression rules diff --git a/src/sksl/SkSLAnalysis.h b/src/sksl/SkSLAnalysis.h index e599abff1d5d..a188a892f307 100644 --- a/src/sksl/SkSLAnalysis.h +++ b/src/sksl/SkSLAnalysis.h @@ -26,6 +26,7 @@ struct Program; class ProgramElement; class ProgramUsage; class Statement; +struct LoopUnrollInfo; class Variable; class VariableReference; enum class VariableRefKind : int8_t; @@ -132,24 +133,17 @@ struct Analysis { // of the texture lookup functions static bool IsConstantExpression(const Expression& expr); - struct UnrollableLoopInfo { - const Variable* fIndex; - double fStart; - double fDelta; - int fCount; - }; - // Ensures that a for-loop meets the strict requirements of The OpenGL ES Shading Language 1.00, // Appendix A, Section 4. - // Information about the loop's structure are placed in outLoopInfo (if not nullptr). - // If the function returns false, specific reasons are reported via errors (if not nullptr). - static bool ForLoopIsValidForES2(int offset, - const Statement* loopInitializer, - const Expression* loopTest, - const Expression* loopNext, - const Statement* loopStatement, - UnrollableLoopInfo* outLoopInfo, - ErrorReporter* errors); + // If the requirements are met, information about the loop's structure is returned. + // If the requirements are not met, the problem is reported via `errors` (if not nullptr), and + // null is returned. + static std::unique_ptr GetLoopUnrollInfo(int offset, + const Statement* loopInitializer, + const Expression* loopTest, + const Expression* loopNext, + const Statement* loopStatement, + ErrorReporter* errors); static void ValidateIndexingForES2(const ProgramElement& pe, ErrorReporter& errors); diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp index 671f69d82fa3..49c193823cd6 100644 --- a/src/sksl/SkSLInliner.cpp +++ b/src/sksl/SkSLInliner.cpp @@ -491,8 +491,10 @@ 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()); + // We can't reuse the unroll info from the original for loop, because it uses a + // different induction variable. Ours is a clone. return ForStatement::Make(*fContext, offset, std::move(initializer), expr(f.test()), - expr(f.next()), stmt(f.statement()), + expr(f.next()), stmt(f.statement()), /*unrollInfo=*/nullptr, SymbolTable::WrapIfBuiltin(f.symbols())); } case Statement::Kind::kIf: { diff --git a/src/sksl/SkSLRehydrator.cpp b/src/sksl/SkSLRehydrator.cpp index 5a1e77254859..35ba9221d780 100644 --- a/src/sksl/SkSLRehydrator.cpp +++ b/src/sksl/SkSLRehydrator.cpp @@ -350,7 +350,7 @@ std::unique_ptr Rehydrator::statement() { std::shared_ptr symbols = this->symbolTable(); return ForStatement::Make(fContext, /*offset=*/-1, std::move(initializer), std::move(test), std::move(next), std::move(body), - std::move(symbols)); + /*unrollInfo=*/nullptr, std::move(symbols)); } case Rehydrator::kIf_Command: { bool isStatic = this->readU8(); diff --git a/src/sksl/codegen/SkSLVMCodeGenerator.cpp b/src/sksl/codegen/SkSLVMCodeGenerator.cpp index d4369c1d47b3..67997de7ec1e 100644 --- a/src/sksl/codegen/SkSLVMCodeGenerator.cpp +++ b/src/sksl/codegen/SkSLVMCodeGenerator.cpp @@ -1459,10 +1459,8 @@ void SkVMGenerator::writeContinueStatement() { void SkVMGenerator::writeForStatement(const ForStatement& f) { // We require that all loops be ES2-compliant (unrollable), and actually unroll them here - Analysis::UnrollableLoopInfo loop; - SkAssertResult(Analysis::ForLoopIsValidForES2(f.fOffset, f.initializer().get(), f.test().get(), - f.next().get(), f.statement().get(), &loop, - /*errors=*/nullptr)); + SkASSERT(f.unrollInfo()); + const LoopUnrollInfo& loop = *f.unrollInfo(); SkASSERT(loop.fIndex->type().slotCount() == 1); size_t indexSlot = this->getSlot(*loop.fIndex); diff --git a/src/sksl/ir/SkSLForStatement.cpp b/src/sksl/ir/SkSLForStatement.cpp index 721bb7b10ff8..2ed9c2bc33f0 100644 --- a/src/sksl/ir/SkSLForStatement.cpp +++ b/src/sksl/ir/SkSLForStatement.cpp @@ -42,12 +42,18 @@ static bool is_simple_initializer(const Statement* stmt) { } std::unique_ptr ForStatement::clone() const { + std::unique_ptr unrollInfo; + if (fUnrollInfo) { + unrollInfo = std::make_unique(*fUnrollInfo); + } + 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(), + std::move(unrollInfo), SymbolTable::WrapIfBuiltin(this->symbols())); } @@ -102,10 +108,11 @@ std::unique_ptr ForStatement::Convert(const Context& context, int off } } + std::unique_ptr unrollInfo; if (context.fConfig->strictES2Mode()) { - if (!Analysis::ForLoopIsValidForES2(offset, initializer.get(), test.get(), next.get(), - statement.get(), /*outLoopInfo=*/nullptr, - context.fErrors)) { + unrollInfo = Analysis::GetLoopUnrollInfo(offset, initializer.get(), test.get(), + next.get(), statement.get(), context.fErrors); + if (!unrollInfo) { return nullptr; } } @@ -121,12 +128,13 @@ std::unique_ptr ForStatement::Convert(const Context& context, int off scope.push_back(std::move(initializer)); scope.push_back(ForStatement::Make(context, offset, /*initializer=*/nullptr, std::move(test), std::move(next), std::move(statement), - std::move(symbolTable))); + std::move(unrollInfo), std::move(symbolTable))); return Block::Make(offset, std::move(scope)); } return ForStatement::Make(context, offset, std::move(initializer), std::move(test), - std::move(next), std::move(statement), std::move(symbolTable)); + std::move(next), std::move(statement), std::move(unrollInfo), + std::move(symbolTable)); } std::unique_ptr ForStatement::ConvertWhile(const Context& context, int offset, @@ -146,18 +154,22 @@ std::unique_ptr ForStatement::Make(const Context& context, int offset std::unique_ptr test, std::unique_ptr next, std::unique_ptr statement, + std::unique_ptr unrollInfo, std::shared_ptr symbolTable) { SkASSERT(is_simple_initializer(initializer.get()) || is_vardecl_block_initializer(initializer.get())); SkASSERT(!test || test->type() == *context.fTypes.fBool); - SkASSERT(!context.fConfig->strictES2Mode() || - Analysis::ForLoopIsValidForES2(offset, initializer.get(), test.get(), next.get(), - statement.get(), /*outLoopInfo=*/nullptr, - /*errors=*/nullptr)); + + // If the caller didn't provide us with unroll info, we can compute it here if needed. + if (!unrollInfo && context.fConfig->strictES2Mode()) { + unrollInfo = Analysis::GetLoopUnrollInfo(offset, initializer.get(), test.get(), + next.get(), statement.get(), /*errors=*/nullptr); + SkASSERT(unrollInfo); + } return std::make_unique(offset, std::move(initializer), std::move(test), std::move(next), std::move(statement), - std::move(symbolTable)); + std::move(unrollInfo), std::move(symbolTable)); } } // namespace SkSL diff --git a/src/sksl/ir/SkSLForStatement.h b/src/sksl/ir/SkSLForStatement.h index 70ecde4a8e48..2f98b8f44296 100644 --- a/src/sksl/ir/SkSLForStatement.h +++ b/src/sksl/ir/SkSLForStatement.h @@ -14,6 +14,16 @@ namespace SkSL { +/** + * The unrollability information for an ES2-compatible loop. + */ +struct LoopUnrollInfo { + const Variable* fIndex; + double fStart; + double fDelta; + int fCount; +}; + /** * A 'for' statement. */ @@ -21,15 +31,20 @@ class ForStatement final : public Statement { public: static constexpr Kind kStatementKind = Kind::kFor; - ForStatement(int offset, std::unique_ptr initializer, - std::unique_ptr test, std::unique_ptr next, - std::unique_ptr statement, std::shared_ptr symbols) - : INHERITED(offset, kStatementKind) - , fSymbolTable(std::move(symbols)) - , fInitializer(std::move(initializer)) - , fTest(std::move(test)) - , fNext(std::move(next)) - , fStatement(std::move(statement)) {} + ForStatement(int offset, + std::unique_ptr initializer, + std::unique_ptr test, + std::unique_ptr next, + std::unique_ptr statement, + std::unique_ptr unrollInfo, + std::shared_ptr symbols) + : INHERITED(offset, kStatementKind) + , fSymbolTable(std::move(symbols)) + , fInitializer(std::move(initializer)) + , fTest(std::move(test)) + , fNext(std::move(next)) + , fStatement(std::move(statement)) + , fUnrollInfo(std::move(unrollInfo)) {} // Creates an SkSL for loop; handles type-coercion and uses the ErrorReporter to report errors. static std::unique_ptr Convert(const Context& context, int offset, @@ -51,6 +66,7 @@ class ForStatement final : public Statement { std::unique_ptr test, std::unique_ptr next, std::unique_ptr statement, + std::unique_ptr unrollInfo, std::shared_ptr symbolTable); std::unique_ptr& initializer() { @@ -89,6 +105,11 @@ class ForStatement final : public Statement { return fSymbolTable; } + /** Loop-unroll information is only supported in strict-ES2 code. Null is returned in ES3+. */ + const LoopUnrollInfo* unrollInfo() const { + return fUnrollInfo.get(); + } + std::unique_ptr clone() const override; String description() const override; @@ -99,6 +120,7 @@ class ForStatement final : public Statement { std::unique_ptr fTest; std::unique_ptr fNext; std::unique_ptr fStatement; + std::unique_ptr fUnrollInfo; using INHERITED = Statement; };