Prune no-op statements with a single traverser
We put pruning literal statements and pruning empty declarations in
the same traverser, as some of the required logic is the same. This
pruning of no-ops is always done as one of the first processing steps
after parsing, so further processing of the AST is simpler.
Since we now prune pure literals before removing no-op cases from the
end of switch statements, we also don't need any sort of special
handling for switch statements in pruning pure literals.
BUG=angleproject:2181
TEST=angle_unittests
Change-Id: I2d86efaeb80baab63ac3cc803f3fd9e7ec02908a
Reviewed-on: https://chromium-review.googlesource.com/727803
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/Compiler.cpp b/src/compiler/translator/Compiler.cpp
index 30b026f..82fc287 100644
--- a/src/compiler/translator/Compiler.cpp
+++ b/src/compiler/translator/Compiler.cpp
@@ -25,7 +25,7 @@
#include "compiler/translator/IsASTDepthBelowLimit.h"
#include "compiler/translator/OutputTree.h"
#include "compiler/translator/ParseContext.h"
-#include "compiler/translator/PruneEmptyDeclarations.h"
+#include "compiler/translator/PruneNoOps.h"
#include "compiler/translator/RegenerateStructNames.h"
#include "compiler/translator/RemoveArrayLengthMethod.h"
#include "compiler/translator/RemoveInvariantDeclaration.h"
@@ -375,17 +375,21 @@
if (success && (compileOptions & SH_LIMIT_EXPRESSION_COMPLEXITY))
success = limitExpressionComplexity(root);
- // Prune empty declarations to work around driver bugs and to keep declaration output
- // simple. We want to do this before most other operations since TIntermDeclaration nodes
- // without any children are tricky to work with.
+ // 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.
+ // After this empty declarations are not allowed in the AST.
if (success)
- PruneEmptyDeclarations(root);
+ PruneNoOps(root);
// In case the last case inside a switch statement is a certain type of no-op, GLSL
// compilers in drivers may not accept it. In this case we clean up the dead code from the
- // end of switch statements. This is also required because PruneEmptyDeclarations may have
- // left switch statements that only contained an empty declaration inside the final case in
- // an invalid state. Relies on that PruneEmptyDeclarations has already been run.
+ // end of switch statements. This is also required because PruneNoOps may have left switch
+ // statements that only contained an empty declaration inside the final case in an invalid
+ // state. Relies on that PruneNoOps has already been run.
if (success)
RemoveNoOpCasesFromEndOfSwitchStatements(root, &symbolTable);
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 603bd86..2170916 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -216,6 +216,7 @@
TIntermTyped *getExpression() { return mExpr; }
TIntermBlock *getBody() { return mBody; }
+ void setInit(TIntermNode *init) { mInit = init; }
void setCondition(TIntermTyped *condition) { mCond = condition; }
void setExpression(TIntermTyped *expression) { mExpr = expression; }
void setBody(TIntermBlock *body) { mBody = body; }
diff --git a/src/compiler/translator/PruneEmptyDeclarations.cpp b/src/compiler/translator/PruneEmptyDeclarations.cpp
deleted file mode 100644
index c763d2e..0000000
--- a/src/compiler/translator/PruneEmptyDeclarations.cpp
+++ /dev/null
@@ -1,120 +0,0 @@
-//
-// Copyright (c) 2002-2015 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.
-//
-// The PruneEmptyDeclarations function prunes unnecessary empty declarations and declarators from
-// the AST.
-
-#include "compiler/translator/PruneEmptyDeclarations.h"
-
-#include "compiler/translator/IntermTraverse.h"
-
-namespace sh
-{
-
-namespace
-{
-
-class PruneEmptyDeclarationsTraverser : private TIntermTraverser
-{
- public:
- static void apply(TIntermBlock *root);
-
- private:
- PruneEmptyDeclarationsTraverser();
- bool visitDeclaration(Visit, TIntermDeclaration *node) override;
-};
-
-void PruneEmptyDeclarationsTraverser::apply(TIntermBlock *root)
-{
- PruneEmptyDeclarationsTraverser prune;
- root->traverse(&prune);
- prune.updateTree();
-}
-
-PruneEmptyDeclarationsTraverser::PruneEmptyDeclarationsTraverser()
- : TIntermTraverser(true, false, false)
-{
-}
-
-bool PruneEmptyDeclarationsTraverser::visitDeclaration(Visit, TIntermDeclaration *node)
-{
- TIntermSequence *sequence = node->getSequence();
- if (sequence->size() >= 1)
- {
- TIntermSymbol *sym = sequence->front()->getAsSymbolNode();
- // Prune declarations without a variable name, unless it's an interface block declaration.
- if (sym != nullptr && sym->getSymbol() == "" && !sym->isInterfaceBlock())
- {
- if (sequence->size() > 1)
- {
- // Generate a replacement that will remove the empty declarator in the beginning of
- // a declarator list. Example of a declaration that will be changed:
- // float, a;
- // will be changed to
- // float a;
- // This applies also to struct declarations.
- TIntermSequence emptyReplacement;
- mMultiReplacements.push_back(
- NodeReplaceWithMultipleEntry(node, sym, emptyReplacement));
- }
- else if (sym->getBasicType() != EbtStruct)
- {
- // Single struct declarations may just declare the struct type and no variables, so
- // they should not be pruned. If there are entirely empty non-struct declarations,
- // they result in TIntermDeclaration nodes without any children in the parsing
- // stage. This will be handled further down in the code.
- UNREACHABLE();
- }
- else if (sym->getType().getQualifier() != EvqGlobal &&
- sym->getType().getQualifier() != EvqTemporary)
- {
- // We've hit an empty struct declaration with a qualifier, for example like
- // this:
- // const struct a { int i; };
- // NVIDIA GL driver version 367.27 doesn't accept this kind of declarations, so
- // we convert the declaration to a regular struct declaration. This is okay,
- // since ESSL 1.00 spec section 4.1.8 says about structs that "The optional
- // qualifiers only apply to any declarators, and are not part of the type being
- // defined for name."
-
- if (mInGlobalScope)
- {
- sym->getTypePointer()->setQualifier(EvqGlobal);
- }
- else
- {
- sym->getTypePointer()->setQualifier(EvqTemporary);
- }
- }
- }
- }
- else
- {
- // We have a declaration with no declarators.
- // The declaration may be either inside a block or in a loop init expression.
- TIntermBlock *parentAsBlock = getParentNode()->getAsBlock();
- ASSERT(parentAsBlock != nullptr || getParentNode()->getAsLoopNode() != nullptr);
- if (parentAsBlock)
- {
- TIntermSequence emptyReplacement;
- mMultiReplacements.push_back(
- NodeReplaceWithMultipleEntry(parentAsBlock, node, emptyReplacement));
- }
- else
- {
- queueReplacement(nullptr, OriginalNode::IS_DROPPED);
- }
- }
- return false;
-}
-
-} // namespace
-
-void PruneEmptyDeclarations(TIntermBlock *root)
-{
- PruneEmptyDeclarationsTraverser::apply(root);
-}
-
-} // namespace sh
diff --git a/src/compiler/translator/PruneEmptyDeclarations.h b/src/compiler/translator/PruneEmptyDeclarations.h
deleted file mode 100644
index 8aa149c..0000000
--- a/src/compiler/translator/PruneEmptyDeclarations.h
+++ /dev/null
@@ -1,19 +0,0 @@
-//
-// Copyright (c) 2002-2015 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.
-//
-// The PruneEmptyDeclarations function prunes unnecessary empty declarations and declarators from
-// the AST.
-
-#ifndef COMPILER_TRANSLATOR_PRUNEEMPTYDECLARATIONS_H_
-#define COMPILER_TRANSLATOR_PRUNEEMPTYDECLARATIONS_H_
-
-namespace sh
-{
-class TIntermBlock;
-
-void PruneEmptyDeclarations(TIntermBlock *root);
-}
-
-#endif // COMPILER_TRANSLATOR_PRUNEEMPTYDECLARATIONS_H_
diff --git a/src/compiler/translator/PruneNoOps.cpp b/src/compiler/translator/PruneNoOps.cpp
new file mode 100644
index 0000000..6c9a02c
--- /dev/null
+++ b/src/compiler/translator/PruneNoOps.cpp
@@ -0,0 +1,156 @@
+//
+// Copyright (c) 2002-2015 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.
+//
+// PruneNoOps.cpp: The PruneNoOps function prunes:
+// 1. Empty declarations "int;". Empty declarators will be pruned as well, so for example:
+// int , a;
+// is turned into
+// 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.
+
+#include "compiler/translator/PruneNoOps.h"
+
+#include "compiler/translator/IntermTraverse.h"
+
+namespace sh
+{
+
+namespace
+{
+
+bool IsNoOp(TIntermNode *node)
+{
+ if (node->getAsConstantUnion() != nullptr)
+ {
+ return true;
+ }
+ bool isEmptyDeclaration = node->getAsDeclarationNode() != nullptr &&
+ node->getAsDeclarationNode()->getSequence()->empty();
+ if (isEmptyDeclaration)
+ {
+ return true;
+ }
+ return false;
+}
+
+class PruneNoOpsTraverser : private TIntermTraverser
+{
+ public:
+ static void apply(TIntermBlock *root);
+
+ private:
+ PruneNoOpsTraverser();
+ bool visitDeclaration(Visit, TIntermDeclaration *node) override;
+ bool visitBlock(Visit visit, TIntermBlock *node) override;
+ bool visitLoop(Visit visit, TIntermLoop *loop) override;
+};
+
+void PruneNoOpsTraverser::apply(TIntermBlock *root)
+{
+ PruneNoOpsTraverser prune;
+ root->traverse(&prune);
+ prune.updateTree();
+}
+
+PruneNoOpsTraverser::PruneNoOpsTraverser() : TIntermTraverser(true, false, false)
+{
+}
+
+bool PruneNoOpsTraverser::visitDeclaration(Visit, TIntermDeclaration *node)
+{
+ TIntermSequence *sequence = node->getSequence();
+ if (sequence->size() >= 1)
+ {
+ TIntermSymbol *sym = sequence->front()->getAsSymbolNode();
+ // Prune declarations without a variable name, unless it's an interface block declaration.
+ if (sym != nullptr && sym->getSymbol() == "" && !sym->isInterfaceBlock())
+ {
+ if (sequence->size() > 1)
+ {
+ // Generate a replacement that will remove the empty declarator in the beginning of
+ // a declarator list. Example of a declaration that will be changed:
+ // float, a;
+ // will be changed to
+ // float a;
+ // This applies also to struct declarations.
+ TIntermSequence emptyReplacement;
+ mMultiReplacements.push_back(
+ NodeReplaceWithMultipleEntry(node, sym, emptyReplacement));
+ }
+ else if (sym->getBasicType() != EbtStruct)
+ {
+ // If there are entirely empty non-struct declarations, they result in
+ // TIntermDeclaration nodes without any children in the parsing stage. These are
+ // handled in visitBlock and visitLoop.
+ UNREACHABLE();
+ }
+ else if (sym->getType().getQualifier() != EvqGlobal &&
+ sym->getType().getQualifier() != EvqTemporary)
+ {
+ // Single struct declarations may just declare the struct type and no variables, so
+ // they should not be pruned. Here we handle an empty struct declaration with a
+ // qualifier, for example like this:
+ // const struct a { int i; };
+ // NVIDIA GL driver version 367.27 doesn't accept this kind of declarations, so we
+ // convert the declaration to a regular struct declaration. This is okay, since ESSL
+ // 1.00 spec section 4.1.8 says about structs that "The optional qualifiers only
+ // apply to any declarators, and are not part of the type being defined for name."
+
+ if (mInGlobalScope)
+ {
+ sym->getTypePointer()->setQualifier(EvqGlobal);
+ }
+ else
+ {
+ sym->getTypePointer()->setQualifier(EvqTemporary);
+ }
+ }
+ }
+ }
+ return false;
+}
+
+bool PruneNoOpsTraverser::visitBlock(Visit visit, TIntermBlock *node)
+{
+ TIntermSequence *statements = node->getSequence();
+
+ for (TIntermNode *statement : *statements)
+ {
+ if (IsNoOp(statement))
+ {
+ TIntermSequence emptyReplacement;
+ mMultiReplacements.push_back(
+ NodeReplaceWithMultipleEntry(node, statement, emptyReplacement));
+ }
+ }
+
+ return true;
+}
+
+bool PruneNoOpsTraverser::visitLoop(Visit visit, TIntermLoop *loop)
+{
+ TIntermTyped *expr = loop->getExpression();
+ if (expr != nullptr && IsNoOp(expr))
+ {
+ loop->setExpression(nullptr);
+ }
+ TIntermNode *init = loop->getInit();
+ if (init != nullptr && IsNoOp(init))
+ {
+ loop->setInit(nullptr);
+ }
+
+ return true;
+}
+
+} // namespace
+
+void PruneNoOps(TIntermBlock *root)
+{
+ PruneNoOpsTraverser::apply(root);
+}
+
+} // namespace sh
diff --git a/src/compiler/translator/PruneNoOps.h b/src/compiler/translator/PruneNoOps.h
new file mode 100644
index 0000000..c421cec
--- /dev/null
+++ b/src/compiler/translator/PruneNoOps.h
@@ -0,0 +1,24 @@
+//
+// Copyright (c) 2002-2015 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.
+//
+// PruneNoOps.h: The PruneNoOps function prunes:
+// 1. Empty declarations "int;". Empty declarators will be pruned as well, so for example:
+// int , a;
+// is turned into
+// 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.
+
+#ifndef COMPILER_TRANSLATOR_PRUNENOOPS_H_
+#define COMPILER_TRANSLATOR_PRUNENOOPS_H_
+
+namespace sh
+{
+class TIntermBlock;
+
+void PruneNoOps(TIntermBlock *root);
+}
+
+#endif // COMPILER_TRANSLATOR_PRUNENOOPS_H_
diff --git a/src/compiler/translator/PrunePureLiteralStatements.cpp b/src/compiler/translator/PrunePureLiteralStatements.cpp
deleted file mode 100644
index 0796f13..0000000
--- a/src/compiler/translator/PrunePureLiteralStatements.cpp
+++ /dev/null
@@ -1,91 +0,0 @@
-//
-// 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.
-//
-// PrunePureLiteralStatements.cpp: Removes statements that are literals and nothing else.
-
-#include "compiler/translator/PrunePureLiteralStatements.h"
-
-#include "compiler/translator/IntermTraverse.h"
-
-namespace sh
-{
-
-namespace
-{
-
-class PrunePureLiteralStatementsTraverser : public TIntermTraverser
-{
- public:
- PrunePureLiteralStatementsTraverser() : TIntermTraverser(true, false, false) {}
-
- bool visitBlock(Visit visit, TIntermBlock *node) override
- {
- TIntermSequence *statements = node->getSequence();
- if (statements == nullptr)
- {
- return false;
- }
-
- // Empty case statements at the end of a switch are invalid: if the last statements
- // of a block was a pure literal, also delete all the case statements directly preceding it.
- bool deleteCaseStatements = false;
- for (int i = static_cast<int>(statements->size()); i-- > 0;)
- {
- TIntermNode *statement = (*statements)[i];
-
- if (statement->getAsConstantUnion() != nullptr)
- {
- TIntermSequence emptyReplacement;
- mMultiReplacements.push_back(
- NodeReplaceWithMultipleEntry(node, statement, emptyReplacement));
-
- if (i == static_cast<int>(statements->size()) - 1)
- {
- deleteCaseStatements = true;
- }
-
- continue;
- }
-
- if (deleteCaseStatements)
- {
- if (statement->getAsCaseNode() != nullptr)
- {
- TIntermSequence emptyReplacement;
- mMultiReplacements.push_back(
- NodeReplaceWithMultipleEntry(node, statement, emptyReplacement));
- }
- else
- {
- deleteCaseStatements = false;
- }
- }
- }
-
- return true;
- }
-
- bool visitLoop(Visit visit, TIntermLoop *loop) override
- {
- TIntermTyped *expr = loop->getExpression();
- if (expr != nullptr && expr->getAsConstantUnion() != nullptr)
- {
- loop->setExpression(nullptr);
- }
-
- return true;
- }
-};
-
-} // namespace
-
-void PrunePureLiteralStatements(TIntermNode *root)
-{
- PrunePureLiteralStatementsTraverser prune;
- root->traverse(&prune);
- prune.updateTree();
-}
-
-} // namespace sh
diff --git a/src/compiler/translator/PrunePureLiteralStatements.h b/src/compiler/translator/PrunePureLiteralStatements.h
deleted file mode 100644
index ef956c0..0000000
--- a/src/compiler/translator/PrunePureLiteralStatements.h
+++ /dev/null
@@ -1,18 +0,0 @@
-//
-// 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.
-//
-// PrunePureLiteralStatements.h: Removes statements that are literals and nothing else.
-
-#ifndef COMPILER_TRANSLATOR_PRUNEPURELITERALSTATEMENTS_H_
-#define COMPILER_TRANSLATOR_PRUNEPURELITERALSTATEMENTS_H_
-
-namespace sh
-{
-class TIntermNode;
-
-void PrunePureLiteralStatements(TIntermNode *root);
-}
-
-#endif // COMPILER_TRANSLATOR_PRUNEPURELITERALSTATEMENTS_H_
diff --git a/src/compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.cpp b/src/compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.cpp
index c11dce3..b86d64d 100644
--- a/src/compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.cpp
+++ b/src/compiler/translator/RemoveNoOpCasesFromEndOfSwitchStatements.cpp
@@ -36,11 +36,13 @@
// here. Note that declarations for struct types do contain a nameless child node.
ASSERT(node->getAsDeclarationNode() == nullptr ||
!node->getAsDeclarationNode()->getSequence()->empty());
+ // Pure literal statements should also already be pruned.
+ ASSERT(node->getAsConstantUnion() == nullptr);
return false;
}
// Return true if all statements in "statements" starting from index i consist only of empty blocks
-// and empty declarations. Returns true also if there are no statements.
+// and no-op statements. Returns true also if there are no statements.
bool AreEmptyBlocks(TIntermSequence *statements, size_t i)
{
for (; i < statements->size(); ++i)
diff --git a/src/compiler/translator/RemoveSwitchFallThrough.cpp b/src/compiler/translator/RemoveSwitchFallThrough.cpp
index ab2045d..dea949f 100644
--- a/src/compiler/translator/RemoveSwitchFallThrough.cpp
+++ b/src/compiler/translator/RemoveSwitchFallThrough.cpp
@@ -97,10 +97,10 @@
void RemoveSwitchFallThroughTraverser::visitConstantUnion(TIntermConstantUnion *node)
{
- // Conditions of case labels are not traversed, so this is some other constant
- // Could be just a statement like "0;"
- mPreviousCase->getSequence()->push_back(node);
- mLastStatementWasBreak = false;
+ // Conditions of case labels are not traversed, so this is a constant statement like "0;".
+ // These are no-ops so there's no need to add them back to the statement list. Should have
+ // already been pruned out of the AST, in fact.
+ UNREACHABLE();
}
bool RemoveSwitchFallThroughTraverser::visitDeclaration(Visit, TIntermDeclaration *node)
diff --git a/src/compiler/translator/TranslatorESSL.cpp b/src/compiler/translator/TranslatorESSL.cpp
index ff4b098..23c967f 100644
--- a/src/compiler/translator/TranslatorESSL.cpp
+++ b/src/compiler/translator/TranslatorESSL.cpp
@@ -8,7 +8,6 @@
#include "compiler/translator/BuiltInFunctionEmulatorGLSL.h"
#include "compiler/translator/EmulatePrecision.h"
-#include "compiler/translator/PrunePureLiteralStatements.h"
#include "compiler/translator/RecordConstantPrecision.h"
#include "compiler/translator/OutputESSL.h"
#include "angle_gl.h"
@@ -34,10 +33,6 @@
ShCompileOptions compileOptions,
PerformanceDiagnostics * /*perfDiagnostics*/)
{
- // The ESSL output doesn't define a default precision for float, so float literal statements
- // end up with no precision which is invalid ESSL.
- PrunePureLiteralStatements(root);
-
TInfoSinkBase &sink = getInfoSink().obj;
int shaderVer = getShaderVersion();