Translator: Prune trivial dead code

src/compiler/translator/Compiler.cpp

@@ -603,12 +603,19 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     // Folding should only be able to generate warnings.
     ASSERT(mDiagnostics.numErrors() == 0);
 
+    // Validate no barrier() after return before prunning it in |PruneNoOps()| below.
+    if (mShaderType == GL_TESS_CONTROL_SHADER && !ValidateBarrierFunctionCall(root, &mDiagnostics))
+    {
+        return false;
+    }
+
     // We prune no-ops to work around driver bugs and to keep AST processing and output simple.
     // The following kinds of no-ops are pruned:
     //   1. Empty declarations "int;".
     //   2. Literal statements: "1.0;". The ESSL output doesn't define a default precision
     //      for float, so float literal statements would end up with no precision which is
     //      invalid ESSL.
+    //   3. Any unreachable statement after a discard, return, break or continue.
     // After this empty declarations are not allowed in the AST.
     if (!PruneNoOps(this, root, &mSymbolTable))
     {
@@ -670,11 +677,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         return false;
     }
 
-    if (mShaderType == GL_TESS_CONTROL_SHADER && !ValidateBarrierFunctionCall(root, &mDiagnostics))
-    {
-        return false;
-    }
-
     // Fail compilation if precision emulation not supported.
     if (getResources().WEBGL_debug_shader_precision && getPragma().debugShaderPrecision &&
         !EmulatePrecision::SupportedInLanguage(mOutputType))

src/compiler/translator/tree_ops/PruneNoOps.cpp

@@ -10,6 +10,7 @@
 //        int a;
 //   2. Literal statements: "1.0;". The ESSL output doesn't define a default precision for float,
 //      so float literal statements would end up with no precision which is invalid ESSL.
+//   3. Statements after discard, return, break and continue.
 
 #include "compiler/translator/tree_ops/PruneNoOps.h"
 
@@ -49,6 +50,9 @@ class PruneNoOpsTraverser : private TIntermTraverser
     bool visitDeclaration(Visit, TIntermDeclaration *node) override;
     bool visitBlock(Visit visit, TIntermBlock *node) override;
     bool visitLoop(Visit visit, TIntermLoop *loop) override;
+    bool visitBranch(Visit visit, TIntermBranch *node) override;
+
+    bool mIsBranchVisited = false;
 };
 
 bool PruneNoOpsTraverser::apply(TCompiler *compiler, TIntermBlock *root, TSymbolTable *symbolTable)
@@ -59,11 +63,16 @@ bool PruneNoOpsTraverser::apply(TCompiler *compiler, TIntermBlock *root, TSymbol
 }
 
 PruneNoOpsTraverser::PruneNoOpsTraverser(TSymbolTable *symbolTable)
-    : TIntermTraverser(true, false, false, symbolTable)
+    : TIntermTraverser(true, true, true, symbolTable)
 {}
 
-bool PruneNoOpsTraverser::visitDeclaration(Visit, TIntermDeclaration *node)
+bool PruneNoOpsTraverser::visitDeclaration(Visit visit, TIntermDeclaration *node)
 {
+    if (visit != PreVisit)
+    {
+        return true;
+    }
+
     TIntermSequence *sequence = node->getSequence();
     if (sequence->size() >= 1)
     {
@@ -127,13 +136,52 @@ bool PruneNoOpsTraverser::visitDeclaration(Visit, TIntermDeclaration *node)
 
 bool PruneNoOpsTraverser::visitBlock(Visit visit, TIntermBlock *node)
 {
+    if (visit == PreVisit)
+    {
+        return true;
+    }
+
     TIntermSequence *statements = node->getSequence();
+    const size_t lastChildIndex = getLastTraversedChildIndex(visit);
+    TIntermSequence emptyReplacement;
+
+    // If a branch is visited, prune the rest of the statements.
+    if (mIsBranchVisited)
+    {
+        for (size_t removeIndex = lastChildIndex + 1; removeIndex < statements->size();
+             ++removeIndex)
+        {
+            TIntermNode *statement = (*statements)[removeIndex];
+
+            // If the statement is a switch case label, stop pruning and continue visiting the
+            // children.
+            if (statement->getAsCaseNode() != nullptr)
+            {
+                mIsBranchVisited = false;
+                return true;
+            }
+
+            mMultiReplacements.emplace_back(node, statement, std::move(emptyReplacement));
+        }
+
+        // If the parent is a block, this is a nested block without any condition (like if, loop or
+        // switch), so the rest of the parent block should also be pruned.  Otherwise the parent
+        // block should be unaffected.
+        if (getParentNode()->getAsBlock() == nullptr)
+        {
+            mIsBranchVisited = false;
+        }
+
+        // Don't visit the pruned children.
+        return false;
+    }
 
-    for (TIntermNode *statement : *statements)
+    // If the statement is a noop, prune it.
+    if (!statements->empty())
     {
+        TIntermNode *statement = (*statements)[lastChildIndex];
         if (IsNoOp(statement))
         {
-            TIntermSequence emptyReplacement;
             mMultiReplacements.emplace_back(node, statement, std::move(emptyReplacement));
         }
     }
@@ -143,6 +191,11 @@ bool PruneNoOpsTraverser::visitBlock(Visit visit, TIntermBlock *node)
 
 bool PruneNoOpsTraverser::visitLoop(Visit visit, TIntermLoop *loop)
 {
+    if (visit != PreVisit)
+    {
+        return true;
+    }
+
     TIntermTyped *expr = loop->getExpression();
     if (expr != nullptr && IsNoOp(expr))
     {
@@ -157,6 +210,15 @@ bool PruneNoOpsTraverser::visitLoop(Visit visit, TIntermLoop *loop)
     return true;
 }
 
+bool PruneNoOpsTraverser::visitBranch(Visit visit, TIntermBranch *node)
+{
+    ASSERT(visit == PreVisit);
+
+    mIsBranchVisited = true;
+
+    // Only possible child is the value of a return statement, which has nothing to prune.
+    return false;
+}
 }  // namespace
 
 bool PruneNoOps(TCompiler *compiler, TIntermBlock *root, TSymbolTable *symbolTable)

src/compiler/translator/tree_ops/PruneNoOps.h

@@ -10,6 +10,7 @@
 //        int a;
 //   2. Literal statements: "1.0;". The ESSL output doesn't define a default precision for float,
 //      so float literal statements would end up with no precision which is invalid ESSL.
+//   3. Statements after discard, return, break and continue.
 
 #ifndef COMPILER_TRANSLATOR_TREEOPS_PRUNENOOPS_H_
 #define COMPILER_TRANSLATOR_TREEOPS_PRUNENOOPS_H_

src/compiler/translator/tree_util/IntermTraverse.cpp

@@ -426,19 +426,26 @@ void TIntermTraverser::traverseFunctionDefinition(TIntermFunctionDefinition *nod
 
     bool visit = true;
 
+    mCurrentChildIndex = 0;
+
     if (preVisit)
         visit = node->visit(PreVisit, this);
 
     if (visit)
     {
+        mCurrentChildIndex = 0;
         node->getFunctionPrototype()->traverse(this);
+        mCurrentChildIndex = 0;
+
         if (inVisit)
             visit = node->visit(InVisit, this);
         if (visit)
         {
-            mInGlobalScope = false;
+            mInGlobalScope     = false;
+            mCurrentChildIndex = 1;
             node->getBody()->traverse(this);
-            mInGlobalScope = true;
+            mCurrentChildIndex = 1;
+            mInGlobalScope     = true;
             if (postVisit)
                 visit = node->visit(PostVisit, this);
         }
@@ -457,6 +464,7 @@ void TIntermTraverser::traverseBlock(TIntermBlock *node)
 
     bool visit = true;
 
+    mCurrentChildIndex        = 0;
     TIntermSequence *sequence = node->getSequence();
 
     if (preVisit)
@@ -464,11 +472,15 @@ void TIntermTraverser::traverseBlock(TIntermBlock *node)
 
     if (visit)
     {
-        for (auto *child : *sequence)
+        for (size_t childIndex = 0; childIndex < sequence->size(); ++childIndex)
         {
+            TIntermNode *child = (*sequence)[childIndex];
             if (visit)
             {
+                mCurrentChildIndex = childIndex;
                 child->traverse(this);
+                mCurrentChildIndex = childIndex;
+
                 if (inVisit)
                 {
                     if (child != sequence->back())

src/tests/gl_tests/GLSLTest.cpp

@@ -9064,6 +9064,109 @@ void main() {
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that dead code after discard, return, continue and branch are pruned.
+TEST_P(GLSLTest_ES3, DeadCodeIsPruned)
+{
+    constexpr char kFS[] = R"(#version 300 es
+precision mediump float;
+out vec4 color;
+
+vec4 f(vec4 c)
+{
+    return c;
+    // dead code
+    c = vec4(0, 0, 1, 1);
+    return c;
+}
+
+void main()
+{
+    vec4 result = vec4(0, 0.5, 0, 1);
+    int var = int(result.y * 2.2);
+
+    {
+        if (result.x > 1.0)
+        {
+            discard;
+            // dead code
+            result = vec4(1, 0, 0, 1);
+        }
+        for (int i = 0; i < 3; ++i)
+        {
+            if (i < 2)
+            {
+                result = f(result);
+                continue;
+                // dead code
+                result = vec4(1, 0, 1, 1);
+            }
+            result = f(result);
+            break;
+            // dead code
+            result = vec4(1, 0, 1, 0);
+        }
+        while (true)
+        {
+            if (result.x > -1.0)
+            {
+                {
+                    result = f(result);
+                    {
+                        break;
+                        // dead code
+                        result = vec4(1, 0, 0, 0);
+                    }
+                    // dead code
+                    for (int j = 0; j < 3; ++j)
+                    {
+                        if (j > 1) continue;
+                        result = vec4(0, 0, 1, 0);
+                        color = vec4(0.5, 0, 0.5, 0.5);
+                        return;
+                    }
+                }
+                // dead code
+                result = vec4(0.5, 0, 0, 0);
+            }
+        }
+        switch (var)
+        {
+        case 2:
+            return;
+            // dead code
+            color = vec4(0.25, 0, 0.25, 0.25);
+        case 1:
+            {
+                // Make sure this path is not pruned due to the return in the previous case.
+                result.y += 0.5;
+                break;
+                // dead code
+                color = vec4(0.25, 0, 0, 0);
+            }
+            // dead code
+            color = vec4(0, 0, 0.25, 0);
+            break;
+        default:
+            break;
+        }
+
+        color = result;
+        return;
+        // dead code
+        color = vec4(0, 0, 0.5, 0);
+    }
+    // dead code
+    color = vec4(0, 0, 0, 0.5);
+})";
+
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Test shader with all resources (default uniform, UBO, SSBO, image, sampler and atomic counter) to
 // make sure they are all linked ok.  The front-end sorts these resources and traverses the list of
 // "uniforms" to find the range for each resource.  A bug there was causing some resource ranges to