moved SkSL Switch data into IRNode

Ethan Nicholas · Skia Commit-Bot · commit 01b05e5b830b · 2020-10-22T20:44:04.000Z
Change-Id: I0373cccfd3acc56417f8d1545bbe7320dc2dfa05 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/327256 Commit-Queue: Ethan Nicholas <ethannicholas@google.com> Reviewed-by: John Stiles <johnstiles@google.com>
diff --git a/src/sksl/SkSLAnalysis.cpp b/src/sksl/SkSLAnalysis.cpp
@@ -482,14 +482,14 @@ bool TProgramVisitor<PROG, EXPR, STMT, ELEM>::visitStatement(STMT s) {
         }
         case Statement::Kind::kSwitch: {
             auto& sw = s.template as<SwitchStatement>();
-            if (this->visitExpression(*sw.fValue)) {
+            if (this->visitExpression(*sw.value())) {
                 return true;
             }
-            for (auto& c : sw.fCases) {
-                if (c->fValue && this->visitExpression(*c->fValue)) {
+            for (const auto& c : sw.cases()) {
+                if (c.value() && this->visitExpression(*c.value())) {
                     return true;
                 }
-                for (auto& st : c->fStatements) {
+                for (auto& st : c.statements()) {
                     if (this->visitStatement(*st)) {
                         return true;
                     }
diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
@@ -612,26 +612,26 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
         }
         case Statement::Kind::kSwitch: {
             SwitchStatement& ss = (*s)->as<SwitchStatement>();
-            this->addExpression(cfg, &ss.fValue, /*constantPropagate=*/true);
+            this->addExpression(cfg, &ss.value(), /*constantPropagate=*/true);
             cfg.currentBlock().fNodes.push_back(BasicBlock::MakeStatement(s));
             BlockId start = cfg.fCurrent;
             BlockId switchExit = cfg.newIsolatedBlock();
             fLoopExits.push(switchExit);
-            for (const auto& c : ss.fCases) {
+            for (auto& c : ss.cases()) {
                 cfg.newBlock();
                 cfg.addExit(start, cfg.fCurrent);
-                if (c->fValue) {
+                if (c.value()) {
                     // technically this should go in the start block, but it doesn't actually matter
                     // because it must be constant. Not worth running two loops for.
-                    this->addExpression(cfg, &c->fValue, /*constantPropagate=*/true);
+                    this->addExpression(cfg, &c.value(), /*constantPropagate=*/true);
                 }
-                for (auto& caseStatement : c->fStatements) {
+                for (auto& caseStatement : c.statements()) {
                     this->addStatement(cfg, &caseStatement);
                 }
             }
             cfg.addExit(cfg.fCurrent, switchExit);
             // note that unlike GLSL, our grammar requires the default case to be last
-            if (ss.fCases.empty() || ss.fCases.back()->fValue) {
+            if (ss.cases().empty() || ss.cases().back().value()) {
                 // switch does not have a default clause, mark that it can skip straight to the end
                 cfg.addExit(start, switchExit);
             }
diff --git a/src/sksl/SkSLCPPCodeGenerator.cpp b/src/sksl/SkSLCPPCodeGenerator.cpp
@@ -380,7 +380,7 @@ void CPPCodeGenerator::writeReturnStatement(const ReturnStatement& s) {
 }
 
 void CPPCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {
-    if (s.fIsStatic) {
+    if (s.isStatic()) {
         this->write("@");
     }
     INHERITED::writeSwitchStatement(s);
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
@@ -1205,9 +1205,9 @@ static std::unique_ptr<Statement> block_for_case(SwitchStatement* switchStatemen
     // We have to be careful to not move any of the pointers until after we're sure we're going to
     // succeed, so before we make any changes at all, we check the switch-cases to decide on a plan
     // of action. First, find the switch-case we are interested in.
-    auto iter = switchStatement->fCases.begin();
-    for (; iter != switchStatement->fCases.end(); ++iter) {
-        if (iter->get() == caseToCapture) {
+    auto iter = switchStatement->cases().begin();
+    for (; iter != switchStatement->cases().end(); ++iter) {
+        if (&*iter == caseToCapture) {
             break;
         }
     }
@@ -1217,8 +1217,8 @@ static std::unique_ptr<Statement> block_for_case(SwitchStatement* switchStatemen
     // statements that we can use for simplification.
     auto startIter = iter;
     Statement* unconditionalBreakStmt = nullptr;
-    for (; iter != switchStatement->fCases.end(); ++iter) {
-        for (std::unique_ptr<Statement>& stmt : (*iter)->fStatements) {
+    for (; iter != switchStatement->cases().end(); ++iter) {
+        for (std::unique_ptr<Statement>& stmt : iter->statements()) {
             if (contains_conditional_break(*stmt)) {
                 // We can't reduce switch-cases to a block when they have conditional breaks.
                 return nullptr;
@@ -1243,7 +1243,7 @@ static std::unique_ptr<Statement> block_for_case(SwitchStatement* switchStatemen
 
     // We can move over most of the statements as-is.
     while (startIter != iter) {
-        for (std::unique_ptr<Statement>& stmt : (*startIter)->fStatements) {
+        for (std::unique_ptr<Statement>& stmt : startIter->statements()) {
             caseStmts.push_back(std::move(stmt));
         }
         ++startIter;
@@ -1252,7 +1252,7 @@ static std::unique_ptr<Statement> block_for_case(SwitchStatement* switchStatemen
     // If we found an unconditional break at the end, we need to move what we can while avoiding
     // that break.
     if (unconditionalBreakStmt != nullptr) {
-        for (std::unique_ptr<Statement>& stmt : (*startIter)->fStatements) {
+        for (std::unique_ptr<Statement>& stmt : startIter->statements()) {
             if (stmt.get() == unconditionalBreakStmt) {
                 move_all_but_break(stmt, &caseStmts);
                 unconditionalBreakStmt = nullptr;
@@ -1266,7 +1266,7 @@ static std::unique_ptr<Statement> block_for_case(SwitchStatement* switchStatemen
     SkASSERT(unconditionalBreakStmt == nullptr);  // Verify that we fixed the unconditional break.
 
     // Return our newly-synthesized block.
-    return std::make_unique<Block>(/*offset=*/-1, std::move(caseStmts), switchStatement->fSymbols);
+    return std::make_unique<Block>(/*offset=*/-1, std::move(caseStmts), switchStatement->symbols());
 }
 
 void Compiler::simplifyStatement(DefinitionMap& definitions,
@@ -1334,28 +1334,30 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
         case Statement::Kind::kSwitch: {
             SwitchStatement& s = stmt->as<SwitchStatement>();
             int64_t switchValue;
-            if (fIRGenerator->getConstantInt(*s.fValue, &switchValue)) {
+            if (fIRGenerator->getConstantInt(*s.value(), &switchValue)) {
                 // switch is constant, replace it with the case that matches
                 bool found = false;
                 SwitchCase* defaultCase = nullptr;
-                for (const std::unique_ptr<SwitchCase>& c : s.fCases) {
-                    if (!c->fValue) {
-                        defaultCase = c.get();
+                for (SwitchCase& c : s.cases()) {
+                    if (!c.value()) {
+                        defaultCase = &c;
                         continue;
                     }
                     int64_t caseValue;
-                    SkAssertResult(fIRGenerator->getConstantInt(*c->fValue, &caseValue));
+                    SkAssertResult(fIRGenerator->getConstantInt(*c.value(), &caseValue));
                     if (caseValue == switchValue) {
-                        std::unique_ptr<Statement> newBlock = block_for_case(&s, c.get());
+                        std::unique_ptr<Statement> newBlock = block_for_case(&s, &c);
                         if (newBlock) {
                             (*iter)->setStatement(std::move(newBlock));
                             found = true;
                             break;
                         } else {
-                            if (s.fIsStatic && !(fFlags & kPermitInvalidStaticTests_Flag)) {
+                            if (s.isStatic() && !(fFlags & kPermitInvalidStaticTests_Flag) &&
+                                optimizationContext->fSilences.find(&s) ==
+                                optimizationContext->fSilences.end()) {
                                 this->error(s.fOffset,
                                             "static switch contains non-static conditional break");
-                                s.fIsStatic = false;
+                                optimizationContext->fSilences.insert(&s);
                             }
                             return; // can't simplify
                         }
@@ -1368,10 +1370,12 @@ void Compiler::simplifyStatement(DefinitionMap& definitions,
                         if (newBlock) {
                             (*iter)->setStatement(std::move(newBlock));
                         } else {
-                            if (s.fIsStatic && !(fFlags & kPermitInvalidStaticTests_Flag)) {
+                            if (s.isStatic() && !(fFlags & kPermitInvalidStaticTests_Flag) &&
+                                optimizationContext->fSilences.find(&s) ==
+                                optimizationContext->fSilences.end()) {
                                 this->error(s.fOffset,
                                             "static switch contains non-static conditional break");
-                                s.fIsStatic = false;
+                                optimizationContext->fSilences.insert(&s);
                             }
                             return; // can't simplify
                         }
@@ -1498,8 +1502,10 @@ bool Compiler::scanCFG(FunctionDefinition& f) {
                         ++iter;
                         break;
                     case Statement::Kind::kSwitch:
-                        if (s.as<SwitchStatement>().fIsStatic &&
-                            !(fFlags & kPermitInvalidStaticTests_Flag)) {
+                        if (s.as<SwitchStatement>().isStatic() &&
+                            !(fFlags & kPermitInvalidStaticTests_Flag) &&
+                            optimizationContext.fSilences.find(&s) ==
+                            optimizationContext.fSilences.end()) {
                             this->error(s.fOffset, "static switch has non-static test");
                         }
                         ++iter;
diff --git a/src/sksl/SkSLDehydrator.cpp b/src/sksl/SkSLDehydrator.cpp
@@ -466,14 +466,14 @@ void Dehydrator::write(const Statement* s) {
             case Statement::Kind::kSwitch: {
                 const SwitchStatement& ss = s->as<SwitchStatement>();
                 this->writeCommand(Rehydrator::kSwitch_Command);
-                this->writeU8(ss.fIsStatic);
-                AutoDehydratorSymbolTable symbols(this, ss.fSymbols);
-                this->write(ss.fValue.get());
-                this->writeU8(ss.fCases.size());
-                for (const std::unique_ptr<SwitchCase>& sc : ss.fCases) {
-                    this->write(sc->fValue.get());
-                    this->writeU8(sc->fStatements.size());
-                    for (const std::unique_ptr<Statement>& stmt : sc->fStatements) {
+                this->writeU8(ss.isStatic());
+                AutoDehydratorSymbolTable symbols(this, ss.symbols());
+                this->write(ss.value().get());
+                this->writeU8(ss.cases().count());
+                for (const SwitchCase& sc : ss.cases()) {
+                    this->write(sc.value().get());
+                    this->writeU8(sc.statements().size());
+                    for (const std::unique_ptr<Statement>& stmt : sc.statements()) {
                         this->write(stmt.get());
                     }
                 }
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -1445,19 +1445,19 @@ void GLSLCodeGenerator::writeDoStatement(const DoStatement& d) {
 
 void GLSLCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {
     this->write("switch (");
-    this->writeExpression(*s.fValue, kTopLevel_Precedence);
+    this->writeExpression(*s.value(), kTopLevel_Precedence);
     this->writeLine(") {");
     fIndentation++;
-    for (const auto& c : s.fCases) {
-        if (c->fValue) {
+    for (const SwitchCase& c : s.cases()) {
+        if (c.value()) {
             this->write("case ");
-            this->writeExpression(*c->fValue, kTopLevel_Precedence);
+            this->writeExpression(*c.value(), kTopLevel_Precedence);
             this->writeLine(":");
         } else {
             this->writeLine("default:");
         }
         fIndentation++;
-        for (const auto& stmt : c->fStatements) {
+        for (const auto& stmt : c.statements()) {
             this->writeStatement(*stmt);
             this->writeLine();
         }
diff --git a/src/sksl/SkSLInliner.cpp b/src/sksl/SkSLInliner.cpp
@@ -535,14 +535,14 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
         case Statement::Kind::kSwitch: {
             const SwitchStatement& ss = statement.as<SwitchStatement>();
             std::vector<std::unique_ptr<SwitchCase>> cases;
-            cases.reserve(ss.fCases.size());
-            for (const auto& sc : ss.fCases) {
-                cases.push_back(std::make_unique<SwitchCase>(offset, expr(sc->fValue),
-                                                             stmts(sc->fStatements)));
+            cases.reserve(ss.cases().count());
+            for (const SwitchCase& sc : ss.cases()) {
+                cases.push_back(std::make_unique<SwitchCase>(offset, expr(sc.value()),
+                                                             stmts(sc.statements())));
             }
-            return std::make_unique<SwitchStatement>(offset, ss.fIsStatic, expr(ss.fValue),
+            return std::make_unique<SwitchStatement>(offset, ss.isStatic(), expr(ss.value()),
                                                      std::move(cases),
-                                                     SymbolTable::WrapIfBuiltin(ss.fSymbols));
+                                                     SymbolTable::WrapIfBuiltin(ss.symbols()));
         }
         case Statement::Kind::kVarDeclaration: {
             const VarDeclaration& decl = statement.as<VarDeclaration>();
@@ -935,14 +935,14 @@ class InlineCandidateAnalyzer {
             }
             case Statement::Kind::kSwitch: {
                 SwitchStatement& switchStmt = (*stmt)->as<SwitchStatement>();
-                if (switchStmt.fSymbols) {
-                    fSymbolTableStack.push_back(switchStmt.fSymbols.get());
+                if (switchStmt.symbols()) {
+                    fSymbolTableStack.push_back(switchStmt.symbols().get());
                 }
 
-                this->visitExpression(&switchStmt.fValue);
-                for (std::unique_ptr<SwitchCase>& switchCase : switchStmt.fCases) {
+                this->visitExpression(&switchStmt.value());
+                for (SwitchCase& switchCase : switchStmt.cases()) {
                     // The switch-case's fValue cannot be a FunctionCall; skip it.
-                    for (std::unique_ptr<Statement>& caseBlock : switchCase->fStatements) {
+                    for (std::unique_ptr<Statement>& caseBlock : switchCase.statements()) {
                         this->visitStatement(&caseBlock);
                     }
                 }
diff --git a/src/sksl/SkSLMetalCodeGenerator.cpp b/src/sksl/SkSLMetalCodeGenerator.cpp
@@ -1365,19 +1365,19 @@ void MetalCodeGenerator::writeDoStatement(const DoStatement& d) {
 
 void MetalCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {
     this->write("switch (");
-    this->writeExpression(*s.fValue, kTopLevel_Precedence);
+    this->writeExpression(*s.value(), kTopLevel_Precedence);
     this->writeLine(") {");
     fIndentation++;
-    for (const auto& c : s.fCases) {
-        if (c->fValue) {
+    for (const SwitchCase& c : s.cases()) {
+        if (c.value()) {
             this->write("case ");
-            this->writeExpression(*c->fValue, kTopLevel_Precedence);
+            this->writeExpression(*c.value(), kTopLevel_Precedence);
             this->writeLine(":");
         } else {
             this->writeLine("default:");
         }
         fIndentation++;
-        for (const auto& stmt : c->fStatements) {
+        for (const auto& stmt : c.statements()) {
             this->writeStatement(*stmt);
             this->writeLine();
         }
@@ -1801,9 +1801,9 @@ MetalCodeGenerator::Requirements MetalCodeGenerator::requirements(const Statemen
         }
         case Statement::Kind::kSwitch: {
             const SwitchStatement& sw = s->as<SwitchStatement>();
-            Requirements result = this->requirements(sw.fValue.get());
-            for (const auto& c : sw.fCases) {
-                for (const auto& st : c->fStatements) {
+            Requirements result = this->requirements(sw.value().get());
+            for (const SwitchCase& sc : sw.cases()) {
+                for (const auto& st : sc.statements()) {
                     result |= this->requirements(st.get());
                 }
             }
diff --git a/src/sksl/SkSLPipelineStageCodeGenerator.cpp b/src/sksl/SkSLPipelineStageCodeGenerator.cpp
@@ -164,7 +164,7 @@ void PipelineStageCodeGenerator::writeIfStatement(const IfStatement& s) {
 }
 
 void PipelineStageCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {
-    if (s.fIsStatic) {
+    if (s.isStatic()) {
         this->write("@");
     }
     INHERITED::writeSwitchStatement(s);
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -3048,16 +3048,17 @@ void SPIRVCodeGenerator::writeDoStatement(const DoStatement& d, OutputStream& ou
 }
 
 void SPIRVCodeGenerator::writeSwitchStatement(const SwitchStatement& s, OutputStream& out) {
-    SpvId value = this->writeExpression(*s.fValue, out);
+    SpvId value = this->writeExpression(*s.value(), out);
     std::vector<SpvId> labels;
     SpvId end = this->nextId();
     SpvId defaultLabel = end;
     fBreakTarget.push(end);
     int size = 3;
-    for (const auto& c : s.fCases) {
+    auto cases = s.cases();
+    for (const SwitchCase& c : cases) {
         SpvId label = this->nextId();
         labels.push_back(label);
-        if (c->fValue) {
+        if (c.value()) {
             size += 2;
         } else {
             defaultLabel = label;
@@ -3068,16 +3069,16 @@ void SPIRVCodeGenerator::writeSwitchStatement(const SwitchStatement& s, OutputSt
     this->writeOpCode(SpvOpSwitch, size, out);
     this->writeWord(value, out);
     this->writeWord(defaultLabel, out);
-    for (size_t i = 0; i < s.fCases.size(); ++i) {
-        if (!s.fCases[i]->fValue) {
+    for (int i = 0; i < cases.count(); ++i) {
+        if (!cases[i].value()) {
             continue;
         }
-        this->writeWord(s.fCases[i]->fValue->as<IntLiteral>().value(), out);
+        this->writeWord(cases[i].value()->as<IntLiteral>().value(), out);
         this->writeWord(labels[i], out);
     }
-    for (size_t i = 0; i < s.fCases.size(); ++i) {
+    for (int i = 0; i < cases.count(); ++i) {
         this->writeLabel(labels[i], out);
-        for (const auto& stmt : s.fCases[i]->fStatements) {
+        for (const auto& stmt : cases[i].statements()) {
             this->writeStatement(*stmt, out);
         }
         if (fCurrentBlock) {
diff --git a/src/sksl/ir/SkSLIRNode.cpp b/src/sksl/ir/SkSLIRNode.cpp
@@ -112,6 +112,11 @@ IRNode::IRNode(int offset, int kind, const String& data)
 , fKind(kind)
 , fData(data) {}
 
+IRNode::IRNode(int offset, int kind, const SwitchStatementData& data)
+: fOffset(offset)
+, fKind(kind)
+, fData(data) {}
+
 IRNode::IRNode(int offset, int kind, const SwizzleData& data)
 : fOffset(offset)
 , fKind(kind)
diff --git a/src/sksl/ir/SkSLIRNode.h b/src/sksl/ir/SkSLIRNode.h
diff --git a/src/sksl/ir/SkSLNodeArrayWrapper.h b/src/sksl/ir/SkSLNodeArrayWrapper.h
diff --git a/src/sksl/ir/SkSLStatement.h b/src/sksl/ir/SkSLStatement.h
diff --git a/src/sksl/ir/SkSLSwitchCase.h b/src/sksl/ir/SkSLSwitchCase.h
diff --git a/src/sksl/ir/SkSLSwitchStatement.h b/src/sksl/ir/SkSLSwitchStatement.h

Original file line number	Diff line number	Diff line change
`@@ -482,14 +482,14 @@ bool TProgramVisitor<PROG, EXPR, STMT, ELEM>::visitStatement(STMT s) {`
`482`	`482`	`}`
`483`	`483`	`case Statement::Kind::kSwitch: {`
`484`	`484`	`auto& sw = s.template as<SwitchStatement>();`
`485`		`- if (this->visitExpression(*sw.fValue)) {`
	`485`	`+ if (this->visitExpression(*sw.value())) {`
`486`	`486`	`return true;`
`487`	`487`	`}`
`488`		`- for (auto& c : sw.fCases) {`
`489`		`- if (c->fValue && this->visitExpression(*c->fValue)) {`
	`488`	`+ for (const auto& c : sw.cases()) {`
	`489`	`+ if (c.value() && this->visitExpression(*c.value())) {`
`490`	`490`	`return true;`
`491`	`491`	`}`
`492`		`- for (auto& st : c->fStatements) {`
	`492`	`+ for (auto& st : c.statements()) {`
`493`	`493`	`if (this->visitStatement(*st)) {`
`494`	`494`	`return true;`
`495`	`495`	`}`
Original file line number	Diff line number	Diff line change
`@@ -380,7 +380,7 @@ void CPPCodeGenerator::writeReturnStatement(const ReturnStatement& s) {`
`380`	`380`	`}`
`381`	`381`
`382`	`382`	`void CPPCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {`
`383`		`- if (s.fIsStatic) {`
	`383`	`+ if (s.isStatic()) {`
`384`	`384`	`this->write("@");`
`385`	`385`	`}`
`386`	`386`	`INHERITED::writeSwitchStatement(s);`
Original file line number	Diff line number	Diff line change
`@@ -164,7 +164,7 @@ void PipelineStageCodeGenerator::writeIfStatement(const IfStatement& s) {`
`164`	`164`	`}`
`165`	`165`
`166`	`166`	`void PipelineStageCodeGenerator::writeSwitchStatement(const SwitchStatement& s) {`
`167`		`- if (s.fIsStatic) {`
	`167`	`+ if (s.isStatic()) {`
`168`	`168`	`this->write("@");`
`169`	`169`	`}`
`170`	`170`	`INHERITED::writeSwitchStatement(s);`