Prune empty cases and switch statements

src/compiler.gypi

@@ -109,6 +109,8 @@
             'compiler/translator/PoolAlloc.cpp',
             'compiler/translator/PoolAlloc.h',
             'compiler/translator/Pragma.h',
+            'compiler/translator/PruneEmptyCases.cpp',
+            'compiler/translator/PruneEmptyCases.h',
             'compiler/translator/PruneNoOps.cpp',
             'compiler/translator/PruneNoOps.h',
             'compiler/translator/QualifierTypes.h',
@@ -119,8 +121,6 @@
             'compiler/translator/RegenerateStructNames.h',
             'compiler/translator/RemoveArrayLengthMethod.cpp',
             'compiler/translator/RemoveArrayLengthMethod.h',
-            'compiler/translator/RemoveEmptySwitchStatements.cpp',
-            'compiler/translator/RemoveEmptySwitchStatements.h',
             'compiler/translator/RemoveInvariantDeclaration.cpp',
             'compiler/translator/RemoveInvariantDeclaration.h',
             'compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.cpp',

src/compiler/translator/Compiler.cpp

@@ -26,10 +26,10 @@
 #include "compiler/translator/IsASTDepthBelowLimit.h"
 #include "compiler/translator/OutputTree.h"
 #include "compiler/translator/ParseContext.h"
+#include "compiler/translator/PruneEmptyCases.h"
 #include "compiler/translator/PruneNoOps.h"
 #include "compiler/translator/RegenerateStructNames.h"
 #include "compiler/translator/RemoveArrayLengthMethod.h"
-#include "compiler/translator/RemoveEmptySwitchStatements.h"
 #include "compiler/translator/RemoveInvariantDeclaration.h"
 #include "compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.h"
 #include "compiler/translator/RemovePow.h"
@@ -473,9 +473,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     // state. Relies on that PruneNoOps has already been run.
     RemoveNoOpCasesFromEndOfSwitchStatements(root, &symbolTable);
 
-    // Remove empty switch statements - this makes output simpler.
-    RemoveEmptySwitchStatements(root);
-
     // Create the function DAG and check there is no recursion
     if (!initCallDag(root))
     {
@@ -588,6 +585,8 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
 
     RemoveUnreferencedVariables(root, &symbolTable);
 
+    PruneEmptyCases(root);
+
     // Built-in function emulation needs to happen after validateLimitations pass.
     // TODO(jmadill): Remove global pool allocator.
     GetGlobalPoolAllocator()->lock();

src/compiler/translator/PruneEmptyCases.cpp

+//
+// Copyright (c) 2018 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// PruneEmptyCases.cpp: The PruneEmptyCases function prunes cases that are followed by nothing from
+// the AST.
+
+#include "compiler/translator/PruneEmptyCases.h"
+
+#include "compiler/translator/IntermTraverse.h"
+#include "compiler/translator/Symbol.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class PruneEmptyCasesTraverser : private TIntermTraverser
+{
+  public:
+    static void apply(TIntermBlock *root);
+
+  private:
+    PruneEmptyCasesTraverser();
+    bool visitSwitch(Visit visit, TIntermSwitch *node) override;
+};
+
+void PruneEmptyCasesTraverser::apply(TIntermBlock *root)
+{
+    PruneEmptyCasesTraverser prune;
+    root->traverse(&prune);
+    prune.updateTree();
+}
+
+PruneEmptyCasesTraverser::PruneEmptyCasesTraverser() : TIntermTraverser(true, false, false)
+{
+}
+
+bool PruneEmptyCasesTraverser::visitSwitch(Visit visit, TIntermSwitch *node)
+{
+    TIntermBlock *statementList = node->getStatementList();
+    TIntermSequence *statements = statementList->getSequence();
+
+    // Iterate block children in reverse order. Cases that are only followed by other cases are
+    // pruned.
+    size_t i                  = statements->size();
+    bool emptySwitchStatement = true;
+    while (i > 0)
+    {
+        --i;
+        TIntermNode *statement = statements->at(i);
+        if (statement->getAsCaseNode())
+        {
+            TIntermSequence emptyReplacement;
+            mMultiReplacements.push_back(
+                NodeReplaceWithMultipleEntry(statementList, statement, emptyReplacement));
+        }
+        else
+        {
+            emptySwitchStatement = false;
+            break;
+        }
+    }
+    if (emptySwitchStatement)
+    {
+        TIntermTyped *init = node->getInit();
+        if (init->hasSideEffects())
+        {
+            queueReplacement(init, OriginalNode::IS_DROPPED);
+        }
+        else
+        {
+            TIntermSequence emptyReplacement;
+            ASSERT(getParentNode()->getAsBlock());
+            mMultiReplacements.push_back(NodeReplaceWithMultipleEntry(getParentNode()->getAsBlock(),
+                                                                      node, emptyReplacement));
+        }
+        return false;
+    }
+
+    return true;
+}
+
+}  // namespace
+
+void PruneEmptyCases(TIntermBlock *root)
+{
+    PruneEmptyCasesTraverser::apply(root);
+}
+
+}  // namespace sh

src/compiler/translator/PruneEmptyCases.h

+//
+// Copyright (c) 2018 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// PruneEmptyCases.h: The PruneEmptyCases function prunes cases that are followed by nothing from
+// the AST.
+
+#ifndef COMPILER_TRANSLATOR_PRUNEEMPTYCASES_H_
+#define COMPILER_TRANSLATOR_PRUNEEMPTYCASES_H_
+
+namespace sh
+{
+class TIntermBlock;
+
+void PruneEmptyCases(TIntermBlock *root);
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_PRUNEEMPTYCASES_H_

src/compiler/translator/RemoveEmptySwitchStatements.cpp

-//
-// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-//
-// RemoveEmptySwitchStatements.cpp: Remove switch statements that have an empty statement list.
-
-#include "compiler/translator/RemoveEmptySwitchStatements.h"
-
-#include "compiler/translator/IntermTraverse.h"
-
-namespace sh
-{
-
-namespace
-{
-
-class RemoveEmptySwitchStatementsTraverser : public TIntermTraverser
-{
-  public:
-    RemoveEmptySwitchStatementsTraverser() : TIntermTraverser(true, false, false) {}
-
-    bool visitSwitch(Visit visit, TIntermSwitch *node);
-};
-
-bool RemoveEmptySwitchStatementsTraverser::visitSwitch(Visit visit, TIntermSwitch *node)
-{
-    if (node->getStatementList()->getSequence()->empty())
-    {
-        // Just output the init statement.
-        if (node->getInit()->hasSideEffects())
-        {
-            queueReplacement(node->getInit(), OriginalNode::IS_DROPPED);
-        }
-        else
-        {
-            TIntermSequence emptyReplacement;
-            ASSERT(getParentNode()->getAsBlock());
-            mMultiReplacements.push_back(NodeReplaceWithMultipleEntry(getParentNode()->getAsBlock(),
-                                                                      node, emptyReplacement));
-        }
-        return false;  // Nothing inside the child nodes to traverse.
-    }
-    return true;
-}
-
-}  // anonymous namespace
-
-void RemoveEmptySwitchStatements(TIntermBlock *root)
-{
-    RemoveEmptySwitchStatementsTraverser traverser;
-    root->traverse(&traverser);
-    traverser.updateTree();
-}
-
-}  // namespace sh

src/compiler/translator/RemoveEmptySwitchStatements.h

-//
-// Copyright (c) 2017 The ANGLE Project Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-//
-// RemoveEmptySwitchStatements.h: Remove switch statements that have an empty statement list.
-
-#ifndef COMPILER_TRANSLATOR_REMOVEEMPTYSWITCHSTATEMENTS_H_
-#define COMPILER_TRANSLATOR_REMOVEEMPTYSWITCHSTATEMENTS_H_
-
-namespace sh
-{
-class TIntermBlock;
-
-void RemoveEmptySwitchStatements(TIntermBlock *root);
-}
-
-#endif  // COMPILER_TRANSLATOR_REMOVEEMPTYSWITCHSTATEMENTS_H_
\ No newline at end of file

src/tests/angle_unittests.gypi

@@ -77,6 +77,7 @@
             '<(angle_path)/src/tests/compiler_tests/NV_draw_buffers_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/OES_standard_derivatives_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/Pack_Unpack_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/PruneEmptyCases_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/PruneEmptyDeclarations_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/PrunePureLiteralStatements_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/PruneUnusedFunctions_test.cpp',

src/tests/compiler_tests/PruneEmptyCases_test.cpp

+//
+// Copyright (c) 2018 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// PruneEmptyCases_test.cpp:
+//   Tests for pruning empty cases and switch statements. This ensures that the translator doesn't
+//   produce switch statements where the last case statement is not followed by anything.
+//
+
+#include "GLSLANG/ShaderLang.h"
+#include "angle_gl.h"
+#include "gtest/gtest.h"
+#include "tests/test_utils/compiler_test.h"
+
+using namespace sh;
+
+namespace
+{
+
+class PruneEmptyCasesTest : public MatchOutputCodeTest
+{
+  public:
+    PruneEmptyCasesTest() : MatchOutputCodeTest(GL_FRAGMENT_SHADER, 0, SH_GLSL_COMPATIBILITY_OUTPUT)
+    {
+    }
+};
+
+// Test that a switch statement that only contains no-ops is pruned entirely.
+TEST_F(PruneEmptyCasesTest, SwitchStatementWithOnlyNoOps)
+{
+    const std::string shaderString =
+        R"(#version 300 es
+
+        uniform int ui;
+
+        void main(void)
+        {
+            int i = ui;
+            switch (i)
+            {
+                case 0:
+                    int j;
+                    1;
+            }
+        })";
+    compile(shaderString);
+    ASSERT_TRUE(notFoundInCode("switch"));
+    ASSERT_TRUE(notFoundInCode("case"));
+}
+
+// Test that a init statement that has a side effect is preserved even if the switch is pruned.
+TEST_F(PruneEmptyCasesTest, SwitchStatementWithOnlyNoOpsAndInitWithSideEffect)
+{
+    const std::string shaderString =
+        R"(#version 300 es
+
+        precision mediump float;
+        out vec4 my_FragColor;
+
+        uniform int uni_i;
+
+        void main(void)
+        {
+            int i = uni_i;
+            switch (++i)
+            {
+                case 0:
+                    int j;
+                    1;
+            }
+            my_FragColor = vec4(i);
+        })";
+    compile(shaderString);
+    ASSERT_TRUE(notFoundInCode("switch"));
+    ASSERT_TRUE(notFoundInCode("case"));
+    ASSERT_TRUE(foundInCode("++_ui"));
+}
+
+// Test a switch statement where the last case only contains no-ops.
+TEST_F(PruneEmptyCasesTest, SwitchStatementLastCaseOnlyNoOps)
+{
+    const std::string shaderString =
+        R"(#version 300 es
+
+        precision mediump float;
+        out vec4 my_FragColor;
+
+        uniform int ui;
+
+        void main(void)
+        {
+            int i = ui;
+            switch (i)
+            {
+                case 0:
+                    my_FragColor = vec4(0);
+                    break;
+                case 1:
+                    int j;
+                    1;
+            }
+        })";
+    compile(shaderString);
+    ASSERT_TRUE(foundInCode("switch"));
+    ASSERT_TRUE(foundInCode("case", 1));
+}
+
+}  // namespace