Wrap switch statements in blocks in HLSL

src/compiler.gypi

@@ -232,6 +232,8 @@
             'compiler/translator/UniformHLSL.h',
             'compiler/translator/UtilsHLSL.cpp',
             'compiler/translator/UtilsHLSL.h',
+            'compiler/translator/WrapSwitchStatementsInBlocks.cpp',
+            'compiler/translator/WrapSwitchStatementsInBlocks.h',
             'compiler/translator/emulated_builtin_functions_hlsl_autogen.cpp',
         ],
         'angle_translator_lib_vulkan_sources':

src/compiler/translator/TranslatorHLSL.cpp

@@ -14,6 +14,7 @@
 #include "compiler/translator/IntermNodePatternMatcher.h"
 #include "compiler/translator/OutputHLSL.h"
 #include "compiler/translator/RemoveDynamicIndexing.h"
+#include "compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.h"
 #include "compiler/translator/RewriteElseBlocks.h"
 #include "compiler/translator/RewriteTexelFetchOffset.h"
 #include "compiler/translator/RewriteUnaryMinusOperatorInt.h"
@@ -23,6 +24,7 @@
 #include "compiler/translator/SimplifyLoopConditions.h"
 #include "compiler/translator/SplitSequenceOperator.h"
 #include "compiler/translator/UnfoldShortCircuitToIf.h"
+#include "compiler/translator/WrapSwitchStatementsInBlocks.h"
 
 namespace sh
 {
@@ -85,6 +87,15 @@ void TranslatorHLSL::translate(TIntermBlock *root, ShCompileOptions compileOptio
     // version and only apply the workaround if it is too old.
     sh::BreakVariableAliasingInInnerLoops(root);
 
+    // WrapSwitchStatementsInBlocks should be called after any AST transformations that might
+    // introduce variable declarations inside the main scope of any switch statement.
+    if (WrapSwitchStatementsInBlocks(root))
+    {
+        // The WrapSwitchStatementsInBlocks step might introduce new no-op cases to the end of
+        // switch statements, so make sure to clean up the AST.
+        RemoveNoOpCasesFromEndOfSwitchStatements(root, &getSymbolTable());
+    }
+
     bool precisionEmulation =
         getResources().WEBGL_debug_shader_precision && getPragma().debugShaderPrecision;
 

src/compiler/translator/WrapSwitchStatementsInBlocks.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.
+//
+// WrapSwitchStatementsInBlocks.cpp: Wrap switch statements in blocks and declare all switch-scoped
+// variables there to make the AST compatible with HLSL output.
+//
+// switch (init)
+// {
+//     case 0:
+//         float f;
+//     default:
+//         f = 1.0;
+// }
+//
+// becomes
+//
+// {
+//     float f;
+//     switch (init)
+//     {
+//         case 0:
+//         default:
+//             f = 1.0;
+//     }
+// }
+
+#include "compiler/translator/WrapSwitchStatementsInBlocks.h"
+
+#include "compiler/translator/IntermNode.h"
+#include "compiler/translator/IntermTraverse.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class WrapSwitchStatementsInBlocksTraverser : public TIntermTraverser
+{
+  public:
+    WrapSwitchStatementsInBlocksTraverser() : TIntermTraverser(true, false, false), mDidWrap(false)
+    {
+    }
+
+    bool visitSwitch(Visit visit, TIntermSwitch *node) override;
+
+    bool didWrap() const { return mDidWrap; }
+
+  private:
+    bool mDidWrap;
+};
+
+bool WrapSwitchStatementsInBlocksTraverser::visitSwitch(Visit, TIntermSwitch *node)
+{
+    std::vector<TIntermDeclaration *> declarations;
+    TIntermSequence *statementList = node->getStatementList()->getSequence();
+    for (TIntermNode *statement : *statementList)
+    {
+        TIntermDeclaration *asDeclaration = statement->getAsDeclarationNode();
+        if (asDeclaration)
+        {
+            declarations.push_back(asDeclaration);
+        }
+    }
+    if (declarations.empty())
+    {
+        // We don't need to wrap the switch if it doesn't contain declarations as its direct
+        // descendants.
+        return true;
+    }
+
+    TIntermBlock *wrapperBlock = new TIntermBlock();
+    for (TIntermDeclaration *declaration : declarations)
+    {
+        // SeparateDeclarations should have already been run.
+        ASSERT(declaration->getSequence()->size() == 1);
+
+        TIntermDeclaration *declarationInBlock = new TIntermDeclaration();
+        TIntermSymbol *declaratorAsSymbol = declaration->getSequence()->at(0)->getAsSymbolNode();
+        if (declaratorAsSymbol)
+        {
+            // This is a simple declaration like: "float f;"
+            // Remove the declaration from inside the switch and put it in the wrapping block.
+            TIntermSequence emptyReplacement;
+            mMultiReplacements.push_back(NodeReplaceWithMultipleEntry(
+                node->getStatementList(), declaration, emptyReplacement));
+
+            declarationInBlock->appendDeclarator(declaratorAsSymbol->deepCopy());
+        }
+        else
+        {
+            // This is an init declaration like: "float f = 0.0;"
+            // Change the init declaration inside the switch into an assignment and put a plain
+            // declaration in the wrapping block.
+            TIntermBinary *declaratorAsBinary =
+                declaration->getSequence()->at(0)->getAsBinaryNode();
+            ASSERT(declaratorAsBinary);
+
+            TIntermBinary *initAssignment = new TIntermBinary(
+                EOpAssign, declaratorAsBinary->getLeft(), declaratorAsBinary->getRight());
+
+            queueReplacementWithParent(node->getStatementList(), declaration, initAssignment,
+                                       OriginalNode::IS_DROPPED);
+
+            declarationInBlock->appendDeclarator(declaratorAsBinary->getLeft()->deepCopy());
+        }
+        wrapperBlock->appendStatement(declarationInBlock);
+    }
+
+    wrapperBlock->appendStatement(node);
+    queueReplacement(wrapperBlock, OriginalNode::BECOMES_CHILD);
+    mDidWrap = true;
+
+    // Should be fine to process multiple switch statements, even nesting ones in the same
+    // traversal.
+    return true;
+}
+
+}  // anonymous namespace
+
+// Wrap switch statements in the AST into blocks when needed.
+bool WrapSwitchStatementsInBlocks(TIntermBlock *root)
+{
+    WrapSwitchStatementsInBlocksTraverser traverser;
+    root->traverse(&traverser);
+    traverser.updateTree();
+    return traverser.didWrap();
+}
+
+}  // namespace sh

src/compiler/translator/WrapSwitchStatementsInBlocks.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.
+//
+// WrapSwitchStatementsInBlocks.h: Wrap switch statements in blocks and declare all switch-scoped
+// variables there to make the AST compatible with HLSL output.
+
+#ifndef COMPILER_TRANSLATOR_WRAPSWITCHSTATEMENTSINBLOCKS_H_
+#define COMPILER_TRANSLATOR_WRAPSWITCHSTATEMENTSINBLOCKS_H_
+
+namespace sh
+{
+
+class TIntermBlock;
+
+// Wrap switch statements in the AST into blocks when needed. Returns true if the AST was changed.
+bool WrapSwitchStatementsInBlocks(TIntermBlock *root);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_WRAPSWITCHSTATEMENTSINBLOCKS_H_

src/tests/gl_tests/GLSLTest.cpp

@@ -3622,6 +3622,75 @@ TEST_P(GLSLTest_ES3, SwitchBreakOrReturnInsideBlocks)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test switch/case where a variable is declared inside one of the cases and is accessed by a
+// subsequent case.
+TEST_P(GLSLTest_ES3, SwitchWithVariableDeclarationInside)
+{
+    const std::string &fragmentShader =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 my_FragColor;
+
+        uniform int u_zero;
+
+        void main()
+        {
+            my_FragColor = vec4(1, 0, 0, 1);
+            switch (u_zero)
+            {
+                case 0:
+                    ivec2 i;
+                    i = ivec2(1, 0);
+                default:
+                    my_FragColor = vec4(0, i[0], 0, 1);
+            }
+        })";
+
+    ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
+// Test nested switch/case where a variable is declared inside one of the cases and is accessed by a
+// subsequent case.
+TEST_P(GLSLTest_ES3, NestedSwitchWithVariableDeclarationInside)
+{
+    const std::string &fragmentShader =
+        R"(#version 300 es
+
+        precision highp float;
+        out vec4 my_FragColor;
+
+        uniform int u_zero;
+        uniform int u_zero2;
+
+        void main()
+        {
+            my_FragColor = vec4(1, 0, 0, 1);
+            switch (u_zero)
+            {
+                case 0:
+                    ivec2 i;
+                    i = ivec2(1, 0);
+                    switch (u_zero2)
+                    {
+                        case 0:
+                            int j;
+                        default:
+                            j = 1;
+                            i *= j;
+                    }
+                default:
+                    my_FragColor = vec4(0, i[0], 0, 1);
+            }
+        })";
+
+    ANGLE_GL_PROGRAM(program, mSimpleVSSource, fragmentShader);
+    drawQuad(program.get(), "inputAttribute", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Use this to select which configurations (e.g. which renderer, which GLES major version) these tests should be run against.
 ANGLE_INSTANTIATE_TEST(GLSLTest,
                        ES2_D3D9(),