Remove unreferenced variables from the AST

src/compiler.gypi

@@ -115,6 +115,8 @@
             'compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.h',
             'compiler/translator/RemovePow.cpp',
             'compiler/translator/RemovePow.h',
+            'compiler/translator/RemoveUnreferencedVariables.cpp',
+            'compiler/translator/RemoveUnreferencedVariables.h',
             'compiler/translator/RewriteDoWhile.cpp',
             'compiler/translator/RewriteDoWhile.h',
             'compiler/translator/RewriteTexelFetchOffset.cpp',

src/compiler/translator/Compiler.cpp

@@ -32,6 +32,7 @@
 #include "compiler/translator/RemoveInvariantDeclaration.h"
 #include "compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.h"
 #include "compiler/translator/RemovePow.h"
+#include "compiler/translator/RemoveUnreferencedVariables.h"
 #include "compiler/translator/RewriteDoWhile.h"
 #include "compiler/translator/ScalarizeVecAndMatConstructorArgs.h"
 #include "compiler/translator/SeparateDeclarations.h"
@@ -468,13 +469,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         return false;
     }
 
-    // Built-in function emulation needs to happen after validateLimitations pass.
-    // TODO(jmadill): Remove global pool allocator.
-    GetGlobalPoolAllocator()->lock();
-    initBuiltInFunctionEmulator(&builtInFunctionEmulator, compileOptions);
-    GetGlobalPoolAllocator()->unlock();
-    builtInFunctionEmulator.markBuiltInFunctionsForEmulation(root);
-
     // Clamping uniform array bounds needs to happen after validateLimitations pass.
     if (compileOptions & SH_CLAMP_INDIRECT_ARRAY_BOUNDS)
     {
@@ -508,6 +502,51 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         RemovePow(root);
     }
 
+    if (compileOptions & SH_REGENERATE_STRUCT_NAMES)
+    {
+        RegenerateStructNames gen(&symbolTable, shaderVersion);
+        root->traverse(&gen);
+    }
+
+    if (shaderType == GL_FRAGMENT_SHADER && shaderVersion == 100 &&
+        compileResources.EXT_draw_buffers && compileResources.MaxDrawBuffers > 1 &&
+        IsExtensionEnabled(extensionBehavior, TExtension::EXT_draw_buffers))
+    {
+        EmulateGLFragColorBroadcast(root, compileResources.MaxDrawBuffers, &outputVariables,
+                                    &symbolTable, shaderVersion);
+    }
+
+    // Split multi declarations and remove calls to array length().
+    // Note that SimplifyLoopConditions needs to be run before any other AST transformations
+    // that may need to generate new statements from loop conditions or loop expressions.
+    SimplifyLoopConditions(
+        root,
+        IntermNodePatternMatcher::kMultiDeclaration | IntermNodePatternMatcher::kArrayLengthMethod,
+        &getSymbolTable(), getShaderVersion());
+
+    // Note that separate declarations need to be run before other AST transformations that
+    // generate new statements from expressions.
+    SeparateDeclarations(root);
+
+    SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod, &getSymbolTable(),
+                          getShaderVersion());
+
+    RemoveArrayLengthMethod(root);
+
+    RemoveUnreferencedVariables(root, &symbolTable);
+
+    // Built-in function emulation needs to happen after validateLimitations pass.
+    // TODO(jmadill): Remove global pool allocator.
+    GetGlobalPoolAllocator()->lock();
+    initBuiltInFunctionEmulator(&builtInFunctionEmulator, compileOptions);
+    GetGlobalPoolAllocator()->unlock();
+    builtInFunctionEmulator.markBuiltInFunctionsForEmulation(root);
+
+    if (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
+    {
+        ScalarizeVecAndMatConstructorArgs(root, shaderType, fragmentPrecisionHigh, &symbolTable);
+    }
+
     if (shouldCollectVariables(compileOptions))
     {
         ASSERT(!variablesCollected);
@@ -536,6 +575,13 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         }
     }
 
+    // Removing invariant declarations must be done after collecting variables.
+    // Otherwise, built-in invariant declarations don't apply.
+    if (RemoveInvariant(shaderType, shaderVersion, outputType, compileOptions))
+    {
+        RemoveInvariantDeclaration(root);
+    }
+
     // gl_Position is always written in compatibility output mode.
     // It may have been already initialized among other output variables, in that case we don't
     // need to initialize it twice.
@@ -546,49 +592,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         mGLPositionInitialized = true;
     }
 
-    // Removing invariant declarations must be done after collecting variables.
-    // Otherwise, built-in invariant declarations don't apply.
-    if (RemoveInvariant(shaderType, shaderVersion, outputType, compileOptions))
-    {
-        sh::RemoveInvariantDeclaration(root);
-    }
-
-    if (compileOptions & SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS)
-    {
-        ScalarizeVecAndMatConstructorArgs(root, shaderType, fragmentPrecisionHigh, &symbolTable);
-    }
-
-    if (compileOptions & SH_REGENERATE_STRUCT_NAMES)
-    {
-        RegenerateStructNames gen(&symbolTable, shaderVersion);
-        root->traverse(&gen);
-    }
-
-    if (shaderType == GL_FRAGMENT_SHADER && shaderVersion == 100 &&
-        compileResources.EXT_draw_buffers && compileResources.MaxDrawBuffers > 1 &&
-        IsExtensionEnabled(extensionBehavior, TExtension::EXT_draw_buffers))
-    {
-        EmulateGLFragColorBroadcast(root, compileResources.MaxDrawBuffers, &outputVariables,
-                                    &symbolTable, shaderVersion);
-    }
-
-    // Split multi declarations and remove calls to array length().
-    // Note that SimplifyLoopConditions needs to be run before any other AST transformations
-    // that may need to generate new statements from loop conditions or loop expressions.
-    SimplifyLoopConditions(
-        root,
-        IntermNodePatternMatcher::kMultiDeclaration | IntermNodePatternMatcher::kArrayLengthMethod,
-        &getSymbolTable(), getShaderVersion());
-
-    // Note that separate declarations need to be run before other AST transformations that
-    // generate new statements from expressions.
-    SeparateDeclarations(root);
-
-    SplitSequenceOperator(root, IntermNodePatternMatcher::kArrayLengthMethod, &getSymbolTable(),
-                          getShaderVersion());
-
-    RemoveArrayLengthMethod(root);
-
     // DeferGlobalInitializers needs to be run before other AST transformations that generate new
     // statements from expressions. But it's fine to run DeferGlobalInitializers after the above
     // SplitSequenceOperator and RemoveArrayLengthMethod since they only have an effect on the AST
@@ -598,7 +601,6 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     bool canUseLoopsToInitialize = !(compileOptions & SH_DONT_USE_LOOPS_TO_INITIALIZE_VARIABLES);
     DeferGlobalInitializers(root, initializeLocalsAndGlobals, canUseLoopsToInitialize, &symbolTable);
 
-
     if (initializeLocalsAndGlobals)
     {
         // Initialize uninitialized local variables.

src/compiler/translator/RemoveUnreferencedVariables.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.
+//
+// RemoveUnreferencedVariables.cpp:
+//  Drop variables that are declared but never referenced in the AST. This avoids adding unnecessary
+//  initialization code for them.
+//
+
+#include "compiler/translator/RemoveUnreferencedVariables.h"
+
+#include "compiler/translator/IntermTraverse.h"
+#include "compiler/translator/SymbolTable.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class CollectVariableRefCountsTraverser : public TIntermTraverser
+{
+  public:
+    CollectVariableRefCountsTraverser();
+
+    using RefCountMap = std::unordered_map<int, unsigned int>;
+    RefCountMap &getSymbolIdRefCounts() { return mSymbolIdRefCounts; }
+
+    void visitSymbol(TIntermSymbol *node) override;
+
+  private:
+    RefCountMap mSymbolIdRefCounts;
+};
+
+CollectVariableRefCountsTraverser::CollectVariableRefCountsTraverser()
+    : TIntermTraverser(true, false, false)
+{
+}
+
+void CollectVariableRefCountsTraverser::visitSymbol(TIntermSymbol *node)
+{
+    auto iter = mSymbolIdRefCounts.find(node->getId());
+    if (iter == mSymbolIdRefCounts.end())
+    {
+        mSymbolIdRefCounts[node->getId()] = 1u;
+        return;
+    }
+    ++(iter->second);
+}
+
+// Traverser that removes all unreferenced variables on one traversal.
+class RemoveUnreferencedVariablesTraverser : public TIntermTraverser
+{
+  public:
+    RemoveUnreferencedVariablesTraverser(
+        CollectVariableRefCountsTraverser::RefCountMap *symbolIdRefCounts,
+        TSymbolTable *symbolTable);
+
+    bool visitDeclaration(Visit visit, TIntermDeclaration *node) override;
+    void visitSymbol(TIntermSymbol *node) override;
+
+    // Traverse loop and block nodes in reverse order. Note that this traverser does not track
+    // parent block positions, so insertStatementInParentBlock is unusable!
+    void traverseBlock(TIntermBlock *block) override;
+    void traverseLoop(TIntermLoop *loop) override;
+
+  private:
+    void removeDeclaration(TIntermDeclaration *node, TIntermTyped *declarator);
+
+    CollectVariableRefCountsTraverser::RefCountMap *mSymbolIdRefCounts;
+    bool mRemoveReferences;
+};
+
+RemoveUnreferencedVariablesTraverser::RemoveUnreferencedVariablesTraverser(
+    CollectVariableRefCountsTraverser::RefCountMap *symbolIdRefCounts,
+    TSymbolTable *symbolTable)
+    : TIntermTraverser(true, false, true, symbolTable),
+      mSymbolIdRefCounts(symbolIdRefCounts),
+      mRemoveReferences(false)
+{
+}
+
+void RemoveUnreferencedVariablesTraverser::removeDeclaration(TIntermDeclaration *node,
+                                                             TIntermTyped *declarator)
+{
+    if (declarator->getType().isStructSpecifier() && !declarator->getType().isNamelessStruct())
+    {
+        // We don't count references to struct types, so if this declaration declares a named struct
+        // type, we'll keep it. We can still change the declarator though so that it doesn't declare
+        // a variable.
+        queueReplacementWithParent(
+            node, declarator,
+            new TIntermSymbol(mSymbolTable->getEmptySymbolId(), TString(""), declarator->getType()),
+            OriginalNode::IS_DROPPED);
+        return;
+    }
+
+    if (getParentNode()->getAsBlock())
+    {
+        TIntermSequence emptyReplacement;
+        mMultiReplacements.push_back(
+            NodeReplaceWithMultipleEntry(getParentNode()->getAsBlock(), node, emptyReplacement));
+    }
+    else
+    {
+        ASSERT(getParentNode()->getAsLoopNode());
+        queueReplacement(nullptr, OriginalNode::IS_DROPPED);
+    }
+}
+
+bool RemoveUnreferencedVariablesTraverser::visitDeclaration(Visit visit, TIntermDeclaration *node)
+{
+    if (visit == PreVisit)
+    {
+        // SeparateDeclarations should have already been run.
+        ASSERT(node->getSequence()->size() == 1u);
+
+        TIntermTyped *declarator = node->getSequence()->back()->getAsTyped();
+        ASSERT(declarator);
+
+        // We can only remove variables that are not a part of the shader interface.
+        TQualifier qualifier = declarator->getQualifier();
+        if (qualifier != EvqTemporary && qualifier != EvqGlobal)
+        {
+            return true;
+        }
+
+        bool canRemove            = false;
+        TIntermSymbol *symbolNode = declarator->getAsSymbolNode();
+        if (symbolNode != nullptr)
+        {
+            canRemove = (*mSymbolIdRefCounts)[symbolNode->getId()] == 1u;
+        }
+        TIntermBinary *initNode = declarator->getAsBinaryNode();
+        if (initNode != nullptr)
+        {
+            ASSERT(initNode->getLeft()->getAsSymbolNode());
+            int symbolId = initNode->getLeft()->getAsSymbolNode()->getId();
+            canRemove =
+                (*mSymbolIdRefCounts)[symbolId] == 1u && !initNode->getRight()->hasSideEffects();
+        }
+
+        if (canRemove)
+        {
+            removeDeclaration(node, declarator);
+            mRemoveReferences = true;
+        }
+        return true;
+    }
+    ASSERT(visit == PostVisit);
+    mRemoveReferences = false;
+    return true;
+}
+
+void RemoveUnreferencedVariablesTraverser::visitSymbol(TIntermSymbol *node)
+{
+    if (mRemoveReferences)
+    {
+        ASSERT(mSymbolIdRefCounts->find(node->getId()) != mSymbolIdRefCounts->end());
+        --(*mSymbolIdRefCounts)[node->getId()];
+    }
+}
+
+void RemoveUnreferencedVariablesTraverser::traverseBlock(TIntermBlock *node)
+{
+    // We traverse blocks in reverse order.  This way reference counts can be decremented when
+    // removing initializers, and variables that become unused when initializers are removed can be
+    // removed on the same traversal.
+
+    ScopedNodeInTraversalPath addToPath(this, node);
+
+    bool visit = true;
+
+    TIntermSequence *sequence = node->getSequence();
+
+    if (preVisit)
+        visit = visitBlock(PreVisit, node);
+
+    if (visit)
+    {
+        for (auto iter = sequence->rbegin(); iter != sequence->rend(); ++iter)
+        {
+            (*iter)->traverse(this);
+            if (visit && inVisit)
+            {
+                if ((iter + 1) != sequence->rend())
+                    visit = visitBlock(InVisit, node);
+            }
+        }
+    }
+
+    if (visit && postVisit)
+        visitBlock(PostVisit, node);
+}
+
+void RemoveUnreferencedVariablesTraverser::traverseLoop(TIntermLoop *node)
+{
+    // We traverse loops in reverse order as well. The loop body gets traversed before the init
+    // node.
+
+    ScopedNodeInTraversalPath addToPath(this, node);
+
+    bool visit = true;
+
+    if (preVisit)
+        visit = visitLoop(PreVisit, node);
+
+    if (visit)
+    {
+        // We don't need to traverse loop expressions or conditions since they can't be declarations
+        // in the AST (loops which have a declaration in their condition get transformed in the
+        // parsing stage).
+        ASSERT(node->getExpression() == nullptr ||
+               node->getExpression()->getAsDeclarationNode() == nullptr);
+        ASSERT(node->getCondition() == nullptr ||
+               node->getCondition()->getAsDeclarationNode() == nullptr);
+
+        if (node->getBody())
+            node->getBody()->traverse(this);
+
+        if (node->getInit())
+            node->getInit()->traverse(this);
+    }
+
+    if (visit && postVisit)
+        visitLoop(PostVisit, node);
+}
+
+}  // namespace
+
+void RemoveUnreferencedVariables(TIntermBlock *root, TSymbolTable *symbolTable)
+{
+    CollectVariableRefCountsTraverser collector;
+    root->traverse(&collector);
+    RemoveUnreferencedVariablesTraverser traverser(&collector.getSymbolIdRefCounts(), symbolTable);
+    root->traverse(&traverser);
+    traverser.updateTree();
+}
+
+}  // namespace sh

src/compiler/translator/RemoveUnreferencedVariables.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.
+//
+// RemoveUnreferencedVariables.h:
+//  Drop variables that are declared but never referenced in the AST. This avoids adding unnecessary
+//  initialization code for them.
+//
+
+#ifndef COMPILER_TRANSLATOR_REMOVEUNREFERENCEDVARIABLES_H_
+#define COMPILER_TRANSLATOR_REMOVEUNREFERENCEDVARIABLES_H_
+
+namespace sh
+{
+
+class TIntermBlock;
+class TSymbolTable;
+
+void RemoveUnreferencedVariables(TIntermBlock *root, TSymbolTable *symbolTable);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_REMOVEUNREFERENCEDVARIABLES_H_

src/tests/angle_unittests.gypi

@@ -79,6 +79,7 @@
             '<(angle_path)/src/tests/compiler_tests/QualificationOrder_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/RecordConstantPrecision_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/RemovePow_test.cpp',
+            '<(angle_path)/src/tests/compiler_tests/RemoveUnreferencedVariables_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/RewriteDoWhile_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/SamplerMultisample_test.cpp',
             '<(angle_path)/src/tests/compiler_tests/ShaderExtension_test.cpp',