commit 91a1053f2d4f7be31d7f3a14f718d0e457e49710
author Ethan Nicholas <ethannicholas@google.com> Thu Jun 22 11:24:38 2017 -0400
committer Skia Commit-Bot <skia-commit-bot@chromium.org> Thu Jun 22 16:53:00 2017 +0000
tree fe67514f77bf6b1b860d3b94aa0fb0430fad9244
parent eb64b9b201d04f9f72b37a05e86d459e6a91d974

Revert "Revert "clean up sksl dead variable handling""

This reverts commit e0a33e28e2786fb17be2edf28e04cd62bfe4ec1e.

Bug: skia:
Change-Id: Iab31ec1ee091aacc3ba854ad91e71840c911f52e
Reviewed-on: https://skia-review.googlesource.com/20506
Reviewed-by: Ethan Nicholas <ethannicholas@google.com>
Commit-Queue: Ethan Nicholas <ethannicholas@google.com>

src/sksl/SkSLCFGGenerator.cpp

diff --git a/src/sksl/SkSLCFGGenerator.cpp b/src/sksl/SkSLCFGGenerator.cpp
index 2fe049d..df10c62 100644
--- a/src/sksl/SkSLCFGGenerator.cpp
+++ b/src/sksl/SkSLCFGGenerator.cpp
@@ -476,6 +476,8 @@
                 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:

src/sksl/SkSLCompiler.cpp

diff --git a/src/sksl/SkSLCompiler.cpp b/src/sksl/SkSLCompiler.cpp
index 2d541a3..39ac315 100644
--- a/src/sksl/SkSLCompiler.cpp
+++ b/src/sksl/SkSLCompiler.cpp
@@ -320,7 +320,9 @@
                 if (s->fKind == Statement::kVarDeclarations_Kind) {
                     const VarDeclarationsStatement* vd = (const VarDeclarationsStatement*) s;
                     for (const auto& decl : vd->fDeclaration->fVars) {
-                        result[((VarDeclaration&) *decl).fVar] = nullptr;
+                        if (decl->fKind == Statement::kVarDeclaration_Kind) {
+                            result[((VarDeclaration&) *decl).fVar] = nullptr;
+                        }
                     }
                 }
             }
@@ -1024,11 +1026,11 @@
     } 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 +1038,36 @@
                         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;
             }
         }
     }

src/sksl/SkSLGLSLCodeGenerator.cpp

diff --git a/src/sksl/SkSLGLSLCodeGenerator.cpp b/src/sksl/SkSLGLSLCodeGenerator.cpp
index 6381acb..4326f4c 100644
--- a/src/sksl/SkSLGLSLCodeGenerator.cpp
+++ b/src/sksl/SkSLGLSLCodeGenerator.cpp
@@ -621,9 +621,6 @@
     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 @@
             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) {

src/sksl/SkSLSPIRVCodeGenerator.cpp

diff --git a/src/sksl/SkSLSPIRVCodeGenerator.cpp b/src/sksl/SkSLSPIRVCodeGenerator.cpp
index c423a22..fda9820 100644
--- a/src/sksl/SkSLSPIRVCodeGenerator.cpp
+++ b/src/sksl/SkSLSPIRVCodeGenerator.cpp
@@ -2677,9 +2677,7 @@
 
 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