clean up sksl dead variable handling

Change-Id: I301e82bf87d976e59a02b0f383da67eaf5a8795a
Reviewed-on: https://skia-review.googlesource.com/18494
Reviewed-by: Chris Dalton <csmartdalton@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

4 files changed, 27 insertions, 8 deletions

diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index 2fe049d9bd..df10c62b7a 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -476,6 +476,8 @@ void CFGGenerator::addStatement(CFG& cfg, std::unique_ptr<Statement>* s) {
                 cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind,
                                                              false, nullptr, &stmt });
             }
+            cfg.fBlocks[cfg.fCurrent].fNodes.push_back({ BasicBlock::Node::kStatement_Kind, false,
+                                                         nullptr, s });
             break;
         }
         case Statement::kDiscard_Kind:
diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 2d541a3b53..0818b75fed 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -1024,11 +1024,11 @@ void Compiler::scanCFG(FunctionDefinition& f) {
     } while (updated);
     ASSERT(!needsRescan);
 
-    // verify static ifs & switches
+    // verify static ifs & switches, clean up dead variable decls
     for (BasicBlock& b : cfg.fBlocks) {
         DefinitionMap definitions = b.fBefore;
 
-        for (auto iter = b.fNodes.begin(); iter != b.fNodes.end() && !needsRescan; ++iter) {
+        for (auto iter = b.fNodes.begin(); iter != b.fNodes.end() && !needsRescan;) {
             if (iter->fKind == BasicBlock::Node::kStatement_Kind) {
                 const Statement& s = **iter->statement();
                 switch (s.fKind) {
@@ -1036,15 +1036,36 @@ void Compiler::scanCFG(FunctionDefinition& f) {
                         if (((const IfStatement&) s).fIsStatic) {
                             this->error(s.fPosition, "static if has non-static test");
                         }
+                        ++iter;
                         break;
                     case Statement::kSwitch_Kind:
                         if (((const SwitchStatement&) s).fIsStatic) {
                             this->error(s.fPosition, "static switch has non-static test");
                         }
+                        ++iter;
                         break;
+                    case Statement::kVarDeclarations_Kind: {
+                        VarDeclarations& decls = *((VarDeclarationsStatement&) s).fDeclaration;
+                        for (auto varIter = decls.fVars.begin(); varIter != decls.fVars.end();) {
+                            if ((*varIter)->fKind == Statement::kNop_Kind) {
+                                varIter = decls.fVars.erase(varIter);
+                            } else {
+                                ++varIter;
+                            }
+                        }
+                        if (!decls.fVars.size()) {
+                            iter = b.fNodes.erase(iter);
+                        } else {
+                            ++iter;
+                        }
+                        break;
+                    }
                     default:
+                        ++iter;
                         break;
                 }
+            } else {
+                ++iter;
             }
         }
     }
diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 6381acb322..4326f4c794 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -621,9 +621,6 @@ void GLSLCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, bool g
     ASSERT(decl.fVars.size() > 0);
     bool wroteType = false;
     for (const auto& stmt : decl.fVars) {
-        if (stmt->fKind == Statement::kNop_Kind) {
-            continue;
-        }
         VarDeclaration& var = (VarDeclaration&) *stmt;
         if (wroteType) {
             this->write(", ");
@@ -838,6 +835,7 @@ bool GLSLCodeGenerator::generateCode() {
             case ProgramElement::kVar_Kind: {
                 VarDeclarations& decl = (VarDeclarations&) *e;
                 if (decl.fVars.size() > 0) {
+                    ASSERT(decl.fVars[0]->fKind == Statement::kVarDeclaration_Kind);
                     int builtin =
                                ((VarDeclaration&) *decl.fVars[0]).fVar->fModifiers.fLayout.fBuiltin;
                     if (builtin == -1) {
diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index c423a221a9..fda98204c0 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2677,9 +2677,7 @@ void SPIRVCodeGenerator::writeGlobalVars(Program::Kind kind, const VarDeclaratio
 
 void SPIRVCodeGenerator::writeVarDeclarations(const VarDeclarations& decl, OutputStream& out) {
     for (const auto& stmt : decl.fVars) {
-        if (stmt->fKind == Statement::kNop_Kind) {
-            continue;
-        }
+        ASSERT(stmt->fKind == Statement::kVarDeclaration_Kind);
         VarDeclaration& varDecl = (VarDeclaration&) *stmt;
         const Variable* var = varDecl.fVar;
         // These haven't been implemented in our SPIR-V generator yet and we only currently use them