Separate expressions returning arrays for HLSL output
Complex array expressions need to be broken down in HLSL output so that they
are built out of simple combinations of operations and symbols.
In practice this means that array constructors, array assignments and functions
that return arrays inside complex expressions need to be replaced by symbols.
After this change, ANGLE passes all dEQP-GLES3.functional.shaders.arrays
tests.
The old SimplifyArrayAssignment stub is removed, the new name
SeparateExpressionsReturningArrays more closely reflects what the function
needs to do.
BUG=angleproject:971, angleproject:941
TEST=dEQP-GLES3.functional.shaders.arrays.*, WebGL 2 conformance tests
Change-Id: Iab8dde72b1126dc2f958ffb5b1b830fe3ce25912
Reviewed-on: https://chromium-review.googlesource.com/272122
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 8b91f51..bdfdd67 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -636,6 +636,11 @@
void incrementParentBlockPos();
void popParentBlock();
+ bool parentNodeIsBlock()
+ {
+ return !mParentBlockStack.empty() && getParentNode() == mParentBlockStack.back().node;
+ }
+
// Return the original name if hash function pointer is NULL;
// otherwise return the hashed name.
static TString hash(const TString& name, ShHashFunction64 hashFunction);
diff --git a/src/compiler/translator/SeparateExpressionsReturningArrays.cpp b/src/compiler/translator/SeparateExpressionsReturningArrays.cpp
new file mode 100644
index 0000000..e787c2f
--- /dev/null
+++ b/src/compiler/translator/SeparateExpressionsReturningArrays.cpp
@@ -0,0 +1,166 @@
+//
+// 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.
+//
+// SeparateExpressionsReturningArrays splits array-returning expressions that are not array names from more complex
+// expressions, assigning them to a temporary variable a#.
+// Examples where a, b and c are all arrays:
+// (a = b) == (a = c) is split into a = b; type[n] a1 = a; a = c; type[n] a2 = a; a1 == a2;
+// type d = type[n](...)[i]; is split into type[n] a1 = type[n](...); type d = a1[i];
+
+#include "compiler/translator/SeparateExpressionsReturningArrays.h"
+
+#include "compiler/translator/IntermNode.h"
+
+namespace
+{
+
+// Traverser that separates one array expression into a statement at a time.
+class SeparateExpressionsTraverser : public TIntermTraverser
+{
+ public:
+ SeparateExpressionsTraverser();
+
+ void traverse(TIntermNode *node);
+ bool visitBinary(Visit visit, TIntermBinary *node) override;
+ bool visitAggregate(Visit visit, TIntermAggregate *node) override;
+
+ void nextIteration();
+ bool foundArrayExpression() const { return mFoundArrayExpression; }
+
+ protected:
+ // Marked to true once an operation that needs to be hoisted out of the expression has been found.
+ // After that, no more AST updates are performed on that traversal.
+ bool mFoundArrayExpression;
+};
+
+SeparateExpressionsTraverser::SeparateExpressionsTraverser()
+ : TIntermTraverser(true, false, false),
+ mFoundArrayExpression(false)
+{
+}
+
+// Performs a shallow copy of an assignment node.
+// These shallow copies are useful when a node gets inserted into an aggregate node
+// and also needs to be replaced in its original location by a different node.
+TIntermBinary *CopyAssignmentNode(TIntermBinary *node)
+{
+ TIntermBinary *copyNode = new TIntermBinary(node->getOp());
+ copyNode->setLeft(node->getLeft());
+ copyNode->setRight(node->getRight());
+ copyNode->setType(node->getType());
+ return copyNode;
+}
+
+// Performs a shallow copy of a constructor/function call node.
+TIntermAggregate *CopyAggregateNode(TIntermAggregate *node)
+{
+ TIntermAggregate *copyNode = new TIntermAggregate(node->getOp());
+ TIntermSequence *copySeq = copyNode->getSequence();
+ copySeq->insert(copySeq->begin(), node->getSequence()->begin(), node->getSequence()->end());
+ copyNode->setType(node->getType());
+ copyNode->setFunctionId(node->getFunctionId());
+ copyNode->setName(node->getName());
+ return copyNode;
+}
+
+bool SeparateExpressionsTraverser::visitBinary(Visit visit, TIntermBinary *node)
+{
+ if (mFoundArrayExpression)
+ return false;
+
+ // Early return if the expression is not an array or if we're not inside a complex expression.
+ if (!node->getType().isArray() || parentNodeIsBlock())
+ return true;
+
+ switch (node->getOp())
+ {
+ case EOpAssign:
+ {
+ mFoundArrayExpression = true;
+
+ TIntermSequence insertions;
+ insertions.push_back(CopyAssignmentNode(node));
+ // TODO(oetuaho): In some cases it would be more optimal to not add the temporary node, but just use the
+ // original target of the assignment. Care must be taken so that this doesn't happen when the same array
+ // symbol is a target of assignment more than once in one expression.
+ insertions.push_back(createTempInitDeclaration(node->getLeft()));
+ insertStatementsInParentBlock(insertions);
+
+ NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()), false);
+ mReplacements.push_back(replaceVariable);
+ }
+ return false;
+ default:
+ return true;
+ }
+}
+
+bool SeparateExpressionsTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
+{
+ if (mFoundArrayExpression)
+ return false; // No need to traverse further
+
+ if (getParentNode() != nullptr)
+ {
+ TIntermBinary *parentBinary = getParentNode()->getAsBinaryNode();
+ bool parentIsAssignment = (parentBinary != nullptr &&
+ (parentBinary->getOp() == EOpAssign || parentBinary->getOp() == EOpInitialize));
+
+ if (!node->getType().isArray() || parentNodeIsBlock() || parentIsAssignment)
+ return true;
+
+ if (node->isConstructor())
+ {
+ mFoundArrayExpression = true;
+
+ TIntermSequence insertions;
+ insertions.push_back(createTempInitDeclaration(CopyAggregateNode(node)));
+ insertStatementsInParentBlock(insertions);
+
+ NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()), false);
+ mReplacements.push_back(replaceVariable);
+
+ return false;
+ }
+ else if (node->getOp() == EOpFunctionCall)
+ {
+ mFoundArrayExpression = true;
+
+ TIntermSequence insertions;
+ insertions.push_back(createTempInitDeclaration(CopyAggregateNode(node)));
+ insertStatementsInParentBlock(insertions);
+
+ NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()), false);
+ mReplacements.push_back(replaceVariable);
+
+ return false;
+ }
+ }
+ return true;
+}
+
+void SeparateExpressionsTraverser::nextIteration()
+{
+ mFoundArrayExpression = false;
+ nextTemporaryIndex();
+}
+
+} // namespace
+
+void SeparateExpressionsReturningArrays(TIntermNode *root, unsigned int *temporaryIndex)
+{
+ SeparateExpressionsTraverser traverser;
+ ASSERT(temporaryIndex != nullptr);
+ traverser.useTemporaryIndex(temporaryIndex);
+ // Separate one expression at a time, and reset the traverser between iterations.
+ do
+ {
+ traverser.nextIteration();
+ root->traverse(&traverser);
+ if (traverser.foundArrayExpression())
+ traverser.updateTree();
+ }
+ while (traverser.foundArrayExpression());
+}
diff --git a/src/compiler/translator/SeparateExpressionsReturningArrays.h b/src/compiler/translator/SeparateExpressionsReturningArrays.h
new file mode 100644
index 0000000..b178ebb
--- /dev/null
+++ b/src/compiler/translator/SeparateExpressionsReturningArrays.h
@@ -0,0 +1,19 @@
+//
+// 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.
+//
+// SeparateExpressionsReturningArrays splits array-returning expressions that are not array names from more complex
+// expressions, assigning them to a temporary variable a#.
+// Examples where a, b and c are all arrays:
+// (a = b) == (a = c) is split into a = b; type[n] a1 = a; a = c; type[n] a2 = a; a1 == a2;
+// type d = type[n](...)[i]; is split into type[n] a1 = type[n](...); type d = a1[i];
+
+#ifndef COMPILER_TRANSLATOR_SEPARATEEXPRESSIONSRETURNINGARRAYS_H_
+#define COMPILER_TRANSLATOR_SEPARATEEXPRESSIONSRETURNINGARRAYS_H_
+
+class TIntermNode;
+
+void SeparateExpressionsReturningArrays(TIntermNode *root, unsigned int *temporaryIndex);
+
+#endif // COMPILER_TRANSLATOR_SEPARATEEXPRESSIONSRETURNINGARRAYS_H_
diff --git a/src/compiler/translator/SimplifyArrayAssignment.cpp b/src/compiler/translator/SimplifyArrayAssignment.cpp
deleted file mode 100644
index ac5eb67..0000000
--- a/src/compiler/translator/SimplifyArrayAssignment.cpp
+++ /dev/null
@@ -1,38 +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.
-//
-
-#include "compiler/translator/SimplifyArrayAssignment.h"
-
-bool SimplifyArrayAssignment::visitBinary(Visit visit, TIntermBinary *node)
-{
- switch (node->getOp())
- {
- case EOpAssign:
- {
- TIntermNode *parent = getParentNode();
- if (node->getLeft()->isArray() && parent != nullptr)
- {
- TIntermAggregate *parentAgg = parent->getAsAggregate();
- if (parentAgg != nullptr && parentAgg->getOp() == EOpSequence)
- {
- // This case is fine, the result of the assignment is not used.
- break;
- }
-
- // The result of the assignment needs to be stored into a temporary variable,
- // the assignment needs to be replaced with a reference to the temporary variable,
- // and the temporary variable needs to finally be assigned to the target variable.
-
- // This also needs to interact correctly with unfolding short circuiting operators.
- UNIMPLEMENTED();
- }
- }
- break;
- default:
- break;
- }
- return true;
-}
diff --git a/src/compiler/translator/SimplifyArrayAssignment.h b/src/compiler/translator/SimplifyArrayAssignment.h
deleted file mode 100644
index 247eb88..0000000
--- a/src/compiler/translator/SimplifyArrayAssignment.h
+++ /dev/null
@@ -1,25 +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.
-//
-// SimplifyArrayAssignment is an AST traverser to replace statements where
-// the return value of array assignment is used with statements where
-// the return value of array assignment is not used.
-//
-
-#ifndef COMPILER_TRANSLATOR_SIMPLIFYARRAYASSIGNMENT_H_
-#define COMPILER_TRANSLATOR_SIMPLIFYARRAYASSIGNMENT_H_
-
-#include "common/angleutils.h"
-#include "compiler/translator/IntermNode.h"
-
-class SimplifyArrayAssignment : public TIntermTraverser
-{
- public:
- SimplifyArrayAssignment() { }
-
- virtual bool visitBinary(Visit visit, TIntermBinary *node);
-};
-
-#endif // COMPILER_TRANSLATOR_SIMPLIFYARRAYASSIGNMENT_H_
diff --git a/src/compiler/translator/TranslatorHLSL.cpp b/src/compiler/translator/TranslatorHLSL.cpp
index 1fffa4c..c90c1c1 100644
--- a/src/compiler/translator/TranslatorHLSL.cpp
+++ b/src/compiler/translator/TranslatorHLSL.cpp
@@ -10,7 +10,7 @@
#include "compiler/translator/OutputHLSL.h"
#include "compiler/translator/SeparateArrayInitialization.h"
#include "compiler/translator/SeparateDeclarations.h"
-#include "compiler/translator/SimplifyArrayAssignment.h"
+#include "compiler/translator/SeparateExpressionsReturningArrays.h"
#include "compiler/translator/UnfoldShortCircuitToIf.h"
TranslatorHLSL::TranslatorHLSL(sh::GLenum type, ShShaderSpec spec, ShShaderOutput output)
@@ -30,12 +30,11 @@
// Note that SeparateDeclarations needs to be run before UnfoldShortCircuitToIf.
UnfoldShortCircuitToIf(root, &temporaryIndex);
+ SeparateExpressionsReturningArrays(root, &temporaryIndex);
+
// Note that SeparateDeclarations needs to be run before SeparateArrayInitialization.
SeparateArrayInitialization(root);
- SimplifyArrayAssignment simplify;
- root->traverse(&simplify);
-
// HLSL doesn't support arrays as return values, we'll need to make functions that have an array
// as a return value to use an out parameter to transfer the array data instead.
ArrayReturnValueToOutParameter(root);