diff --git a/resources/sksl/errors/StaticSwitchTest.sksl b/resources/sksl/errors/StaticSwitchTest.sksl index 338f20c5c9..bf71f864f6 100644 --- a/resources/sksl/errors/StaticSwitchTest.sksl +++ b/resources/sksl/errors/StaticSwitchTest.sksl @@ -1,10 +1,9 @@ -void non_constant_test_in_static_switch() { +half4 main() { int x = int(sqrt(1)); @switch (x) { case 1: - sk_FragColor = half4(1); - break; + return half4(1); default: - sk_FragColor = half4(0); + return half4(0); } } diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp index 40800b691d..c7785984a5 100644 --- a/src/sksl/SkSLCompiler.cpp +++ b/src/sksl/SkSLCompiler.cpp @@ -1171,14 +1171,14 @@ static bool contains_conditional_break(Statement& stmt) { bool visitStatement(const Statement& stmt) override { switch (stmt.kind()) { case Statement::Kind::kBlock: - return this->INHERITED::visitStatement(stmt); + return INHERITED::visitStatement(stmt); case Statement::Kind::kBreak: return fInConditional > 0; case Statement::Kind::kIf: { ++fInConditional; - bool result = this->INHERITED::visitStatement(stmt); + bool result = INHERITED::visitStatement(stmt); --fInConditional; return result; } @@ -1203,7 +1203,7 @@ static bool contains_unconditional_break(Statement& stmt) { bool visitStatement(const Statement& stmt) override { switch (stmt.kind()) { case Statement::Kind::kBlock: - return this->INHERITED::visitStatement(stmt); + return INHERITED::visitStatement(stmt); case Statement::Kind::kBreak: return true; @@ -1552,39 +1552,6 @@ bool Compiler::scanCFG(FunctionDefinition& f, ProgramUsage* usage) { } while (optimizationContext.fUpdated); SkASSERT(!optimizationContext.fNeedsRescan); - // verify static ifs & switches, clean up dead variable decls - for (BasicBlock& b : cfg.fBlocks) { - for (auto iter = b.fNodes.begin(); iter != b.fNodes.end() && - !optimizationContext.fNeedsRescan;) { - if (iter->isStatement()) { - const Statement& s = **iter->statement(); - switch (s.kind()) { - case Statement::Kind::kIf: - if (s.as().isStatic() && - !fIRGenerator->fSettings->fPermitInvalidStaticTests) { - this->error(s.fOffset, "static if has non-static test"); - } - ++iter; - break; - case Statement::Kind::kSwitch: - if (s.as().isStatic() && - !fIRGenerator->fSettings->fPermitInvalidStaticTests && - optimizationContext.fSilences.find(&s) == - optimizationContext.fSilences.end()) { - this->error(s.fOffset, "static switch has non-static test"); - } - ++iter; - break; - default: - ++iter; - break; - } - } else { - ++iter; - } - } - } - // check for missing return if (f.declaration().returnType() != *fContext->fTypes.fVoid) { if (cfg.fBlocks[cfg.fExit].fIsReachable) { @@ -1652,6 +1619,52 @@ std::unique_ptr Compiler::convertProgram( return success ? std::move(program) : nullptr; } +void Compiler::verifyStaticTests(const Program& program) { + class StaticTestVerifier : public ProgramVisitor { + public: + StaticTestVerifier(ErrorReporter* r) : fReporter(r) {} + + using ProgramVisitor::visitProgramElement; + + bool visitStatement(const Statement& stmt) override { + switch (stmt.kind()) { + case Statement::Kind::kIf: + if (stmt.as().isStatic()) { + fReporter->error(stmt.fOffset, "static if has non-static test"); + } + break; + + case Statement::Kind::kSwitch: + if (stmt.as().isStatic()) { + fReporter->error(stmt.fOffset, "static switch has non-static test"); + } + break; + + default: + break; + } + return INHERITED::visitStatement(stmt); + } + + private: + using INHERITED = ProgramVisitor; + ErrorReporter* fReporter; + }; + + // If invalid static tests are permitted, we don't need to check anything. + if (fIRGenerator->fSettings->fPermitInvalidStaticTests) { + return; + } + + // Check all of the program's owned elements. (Built-in elements are assumed to be valid.) + StaticTestVerifier visitor{this}; + for (const std::unique_ptr& element : program.ownedElements()) { + if (element->is()) { + visitor.visitProgramElement(*element); + } + } +} + bool Compiler::optimize(LoadedModule& module) { SkASSERT(!fErrorCount); Program::Settings settings; @@ -1757,6 +1770,11 @@ bool Compiler::optimize(Program& program) { break; } } + + if (fErrorCount == 0) { + this->verifyStaticTests(program); + } + return fErrorCount == 0; } diff --git a/src/sksl/SkSLCompiler.h b/src/sksl/SkSLCompiler.h index ae6b848c5b..0bf7b04923 100644 --- a/src/sksl/SkSLCompiler.h +++ b/src/sksl/SkSLCompiler.h @@ -181,6 +181,9 @@ private: void scanCFG(CFG* cfg, BlockId block, SkBitSet* processedSet); void computeDataFlow(CFG* cfg); + /** Verifies that @if and @switch statements were actually optimized away. */ + void verifyStaticTests(const Program& program); + /** * Simplifies the expression pointed to by iter (in both the IR and CFG structures), if * possible. diff --git a/src/sksl/ir/SkSLProgram.h b/src/sksl/ir/SkSLProgram.h index ae0aa01b99..b8ff084ad7 100644 --- a/src/sksl/ir/SkSLProgram.h +++ b/src/sksl/ir/SkSLProgram.h @@ -296,7 +296,7 @@ struct Program { // Can be used to iterate over *just* the elements owned by the Program, not shared builtins. // The iterator's value type is 'std::unique_ptr', and mutation is allowed. - const std::vector>& ownedElements() { return fElements; } + const std::vector>& ownedElements() const { return fElements; } Kind fKind; std::unique_ptr fSource;