Separate expressions returning arrays for HLSL output

src/compiler.gypi

@@ -161,8 +161,8 @@
             'compiler/translator/SeparateArrayInitialization.h',
             'compiler/translator/SeparateDeclarations.cpp',
             'compiler/translator/SeparateDeclarations.h',
-            'compiler/translator/SimplifyArrayAssignment.cpp',
-            'compiler/translator/SimplifyArrayAssignment.h',
+            'compiler/translator/SeparateExpressionsReturningArrays.cpp',
+            'compiler/translator/SeparateExpressionsReturningArrays.h',
             'compiler/translator/StructureHLSL.cpp',
             'compiler/translator/StructureHLSL.h',
             'compiler/translator/TranslatorHLSL.cpp',

src/compiler/translator/IntermNode.h

@@ -636,6 +636,11 @@ class TIntermTraverser : angle::NonCopyable
     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);

src/compiler/translator/SeparateExpressionsReturningArrays.cpp

+//
+// 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());
+}

src/compiler/translator/SeparateExpressionsReturningArrays.h

+//
+// 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_

src/compiler/translator/SimplifyArrayAssignment.cpp

-//
-// 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;
-}

src/compiler/translator/SimplifyArrayAssignment.h

-//
-// 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_

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 @@ void TranslatorHLSL::translate(TIntermNode *root, int compileOptions)
     // 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);