Add IntermNodePatternMatcher helper class

src/compiler.gypi

@@ -168,6 +168,8 @@
             'compiler/translator/blocklayoutHLSL.h',
             'compiler/translator/BuiltInFunctionEmulatorHLSL.cpp',
             'compiler/translator/BuiltInFunctionEmulatorHLSL.h',
+            'compiler/translator/IntermNodePatternMatcher.cpp',
+            'compiler/translator/IntermNodePatternMatcher.h',
             'compiler/translator/OutputHLSL.cpp',
             'compiler/translator/OutputHLSL.h',
             'compiler/translator/RemoveDynamicIndexing.cpp',

src/compiler/translator/IntermNodePatternMatcher.cpp

+//
+// Copyright (c) 2016 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.
+//
+// IntermNodePatternMatcher is a helper class for matching node trees to given patterns.
+// It can be used whenever the same checks for certain node structures are common to multiple AST
+// traversers.
+//
+
+#include "compiler/translator/IntermNodePatternMatcher.h"
+
+#include "compiler/translator/IntermNode.h"
+
+namespace
+{
+
+bool IsNodeBlock(TIntermNode *node)
+{
+    ASSERT(node != nullptr);
+    return (node->getAsAggregate() && node->getAsAggregate()->getOp() == EOpSequence);
+}
+
+}  // anonymous namespace
+
+IntermNodePatternMatcher::IntermNodePatternMatcher(const unsigned int mask) : mMask(mask)
+{
+}
+
+bool IntermNodePatternMatcher::match(TIntermBinary *node, TIntermNode *parentNode)
+{
+    if ((mMask & kExpressionReturningArray) != 0)
+    {
+        if (node->isArray() && node->getOp() == EOpAssign && parentNode != nullptr &&
+            !IsNodeBlock(parentNode))
+        {
+            return true;
+        }
+    }
+
+    if ((mMask & kUnfoldedShortCircuitExpression) != 0)
+    {
+        if (node->getRight()->hasSideEffects() &&
+            (node->getOp() == EOpLogicalOr || node->getOp() == EOpLogicalAnd))
+        {
+            return true;
+        }
+    }
+    return false;
+}
+
+bool IntermNodePatternMatcher::match(TIntermAggregate *node, TIntermNode *parentNode)
+{
+    if ((mMask & kExpressionReturningArray) != 0)
+    {
+        if (parentNode != nullptr)
+        {
+            TIntermBinary *parentBinary = parentNode->getAsBinaryNode();
+            bool parentIsAssignment =
+                (parentBinary != nullptr &&
+                 (parentBinary->getOp() == EOpAssign || parentBinary->getOp() == EOpInitialize));
+
+            if (node->getType().isArray() && !parentIsAssignment &&
+                (node->isConstructor() || node->getOp() == EOpFunctionCall) &&
+                !IsNodeBlock(parentNode))
+            {
+                return true;
+            }
+        }
+    }
+    return false;
+}
+
+bool IntermNodePatternMatcher::match(TIntermSelection *node)
+{
+    if ((mMask & kUnfoldedShortCircuitExpression) != 0)
+    {
+        if (node->usesTernaryOperator())
+        {
+            return true;
+        }
+    }
+    return false;
+}

src/compiler/translator/IntermNodePatternMatcher.h

+//
+// Copyright (c) 2016 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.
+//
+// IntermNodePatternMatcher is a helper class for matching node trees to given patterns.
+// It can be used whenever the same checks for certain node structures are common to multiple AST
+// traversers.
+//
+
+#ifndef COMPILER_TRANSLATOR_INTERMNODEPATTERNMATCHER_H_
+#define COMPILER_TRANSLATOR_INTERMNODEPATTERNMATCHER_H_
+
+class TIntermAggregate;
+class TIntermBinary;
+class TIntermNode;
+class TIntermSelection;
+
+class IntermNodePatternMatcher
+{
+  public:
+    enum PatternType
+    {
+        kUnfoldedShortCircuitExpression = 0x0001,
+
+        // Matches expressions that return arrays with the exception of simple statements where a
+        // constructor or function call result is assigned.
+        kExpressionReturningArray = 0x0002
+    };
+    IntermNodePatternMatcher(const unsigned int mask);
+
+    bool match(TIntermBinary *node, TIntermNode *parentNode);
+    bool match(TIntermAggregate *node, TIntermNode *parentNode);
+    bool match(TIntermSelection *node);
+
+  private:
+    const unsigned int mMask;
+};
+
+#endif

src/compiler/translator/SeparateExpressionsReturningArrays.cpp

@@ -12,6 +12,7 @@
 #include "compiler/translator/SeparateExpressionsReturningArrays.h"
 
 #include "compiler/translator/IntermNode.h"
+#include "compiler/translator/IntermNodePatternMatcher.h"
 
 namespace
 {
@@ -32,11 +33,14 @@ class SeparateExpressionsTraverser : public TIntermTraverser
     // 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;
+
+    IntermNodePatternMatcher mPatternToSeparateMatcher;
 };
 
 SeparateExpressionsTraverser::SeparateExpressionsTraverser()
     : TIntermTraverser(true, false, false),
-      mFoundArrayExpression(false)
+      mFoundArrayExpression(false),
+      mPatternToSeparateMatcher(IntermNodePatternMatcher::kExpressionReturningArray)
 {
 }
 
@@ -73,31 +77,27 @@ 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 if the expression is not an array or if we're not inside a complex expression.
+    if (!mPatternToSeparateMatcher.match(node, getParentNode()))
         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;
-    }
+    ASSERT(node->getOp() == 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;
 }
 
 bool SeparateExpressionsTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
@@ -105,43 +105,22 @@ bool SeparateExpressionsTraverser::visitAggregate(Visit visit, TIntermAggregate
     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);
+    if (!mPatternToSeparateMatcher.match(node, getParentNode()))
+        return true;
 
-            NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()), false);
-            mReplacements.push_back(replaceVariable);
+    ASSERT(node->isConstructor() || node->getOp() == EOpFunctionCall);
 
-            return false;
-        }
-        else if (node->getOp() == EOpFunctionCall)
-        {
-            mFoundArrayExpression = true;
+    mFoundArrayExpression = true;
 
-            TIntermSequence insertions;
-            insertions.push_back(createTempInitDeclaration(CopyAggregateNode(node)));
-            insertStatementsInParentBlock(insertions);
+    TIntermSequence insertions;
+    insertions.push_back(createTempInitDeclaration(CopyAggregateNode(node)));
+    insertStatementsInParentBlock(insertions);
 
-            NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()), false);
-            mReplacements.push_back(replaceVariable);
+    NodeUpdateEntry replaceVariable(getParentNode(), node, createTempSymbol(node->getType()),
+                                    false);
+    mReplacements.push_back(replaceVariable);
 
-            return false;
-        }
-    }
-    return true;
+    return false;
 }
 
 void SeparateExpressionsTraverser::nextIteration()

src/compiler/translator/UnfoldShortCircuitToIf.cpp

@@ -11,6 +11,7 @@
 #include "compiler/translator/UnfoldShortCircuitToIf.h"
 
 #include "compiler/translator/IntermNode.h"
+#include "compiler/translator/IntermNodePatternMatcher.h"
 
 namespace
 {
@@ -48,6 +49,8 @@ class UnfoldShortCircuitTraverser : public TIntermTraverser
 
     bool mInLoopCondition;
     bool mInLoopExpression;
+
+    IntermNodePatternMatcher mPatternToUnfoldMatcher;
 };
 
 UnfoldShortCircuitTraverser::UnfoldShortCircuitTraverser()
@@ -56,7 +59,8 @@ UnfoldShortCircuitTraverser::UnfoldShortCircuitTraverser()
       mParentLoop(nullptr),
       mLoopParent(nullptr),
       mInLoopCondition(false),
-      mInLoopExpression(false)
+      mInLoopExpression(false),
+      mPatternToUnfoldMatcher(IntermNodePatternMatcher::kUnfoldedShortCircuitExpression)
 {
 }
 
@@ -64,18 +68,23 @@ bool UnfoldShortCircuitTraverser::visitBinary(Visit visit, TIntermBinary *node)
 {
     if (mFoundShortCircuit)
         return false;
+
+    if (visit != PreVisit)
+        return true;
+
+    if (!mPatternToUnfoldMatcher.match(node, getParentNode()))
+        return true;
+
     // If our right node doesn't have side effects, we know we don't need to unfold this
     // expression: there will be no short-circuiting side effects to avoid
     // (note: unfolding doesn't depend on the left node -- it will always be evaluated)
-    if (!node->getRight()->hasSideEffects())
-    {
-        return true;
-    }
+    ASSERT(node->getRight()->hasSideEffects());
+
+    mFoundShortCircuit = true;
 
     switch (node->getOp())
     {
       case EOpLogicalOr:
-        mFoundShortCircuit = true;
         if (!copyLoopConditionOrExpression(getParentNode(), node))
         {
             // "x || y" is equivalent to "x ? true : y", which unfolds to "bool s; if(x) s = true;
@@ -104,7 +113,6 @@ bool UnfoldShortCircuitTraverser::visitBinary(Visit visit, TIntermBinary *node)
         }
         return false;
       case EOpLogicalAnd:
-        mFoundShortCircuit = true;
         if (!copyLoopConditionOrExpression(getParentNode(), node))
         {
             // "x && y" is equivalent to "x ? y : false", which unfolds to "bool s; if(x) s = y;
@@ -130,7 +138,8 @@ bool UnfoldShortCircuitTraverser::visitBinary(Visit visit, TIntermBinary *node)
         }
         return false;
       default:
-        return true;
+          UNREACHABLE();
+          return true;
     }
 }
 
@@ -139,43 +148,46 @@ bool UnfoldShortCircuitTraverser::visitSelection(Visit visit, TIntermSelection *
     if (mFoundShortCircuit)
         return false;
 
+    if (visit != PreVisit)
+        return true;
+
+    if (!mPatternToUnfoldMatcher.match(node))
+        return true;
+
+    mFoundShortCircuit = true;
+
+    ASSERT(node->usesTernaryOperator());
+
     // Unfold "b ? x : y" into "type s; if(b) s = x; else s = y;"
-    if (visit == PreVisit && node->usesTernaryOperator())
+    if (!copyLoopConditionOrExpression(getParentNode(), node))
     {
-        mFoundShortCircuit = true;
-        if (!copyLoopConditionOrExpression(getParentNode(), node))
-        {
-            TIntermSequence insertions;
+        TIntermSequence insertions;
 
-            TIntermSymbol *tempSymbol         = createTempSymbol(node->getType());
-            TIntermAggregate *tempDeclaration = new TIntermAggregate(EOpDeclaration);
-            tempDeclaration->getSequence()->push_back(tempSymbol);
-            insertions.push_back(tempDeclaration);
+        TIntermSymbol *tempSymbol         = createTempSymbol(node->getType());
+        TIntermAggregate *tempDeclaration = new TIntermAggregate(EOpDeclaration);
+        tempDeclaration->getSequence()->push_back(tempSymbol);
+        insertions.push_back(tempDeclaration);
 
-            TIntermAggregate *trueBlock = new TIntermAggregate(EOpSequence);
-            TIntermBinary *trueAssignment =
-                createTempAssignment(node->getTrueBlock()->getAsTyped());
-            trueBlock->getSequence()->push_back(trueAssignment);
+        TIntermAggregate *trueBlock   = new TIntermAggregate(EOpSequence);
+        TIntermBinary *trueAssignment = createTempAssignment(node->getTrueBlock()->getAsTyped());
+        trueBlock->getSequence()->push_back(trueAssignment);
 
-            TIntermAggregate *falseBlock = new TIntermAggregate(EOpSequence);
-            TIntermBinary *falseAssignment =
-                createTempAssignment(node->getFalseBlock()->getAsTyped());
-            falseBlock->getSequence()->push_back(falseAssignment);
+        TIntermAggregate *falseBlock   = new TIntermAggregate(EOpSequence);
+        TIntermBinary *falseAssignment = createTempAssignment(node->getFalseBlock()->getAsTyped());
+        falseBlock->getSequence()->push_back(falseAssignment);
 
-            TIntermSelection *ifNode =
-                new TIntermSelection(node->getCondition()->getAsTyped(), trueBlock, falseBlock);
-            insertions.push_back(ifNode);
+        TIntermSelection *ifNode =
+            new TIntermSelection(node->getCondition()->getAsTyped(), trueBlock, falseBlock);
+        insertions.push_back(ifNode);
 
-            insertStatementsInParentBlock(insertions);
+        insertStatementsInParentBlock(insertions);
 
-            TIntermSymbol *ternaryResult = createTempSymbol(node->getType());
-            NodeUpdateEntry replaceVariable(getParentNode(), node, ternaryResult, false);
-            mReplacements.push_back(replaceVariable);
-        }
-        return false;
+        TIntermSymbol *ternaryResult = createTempSymbol(node->getType());
+        NodeUpdateEntry replaceVariable(getParentNode(), node, ternaryResult, false);
+        mReplacements.push_back(replaceVariable);
     }
 
-    return true;
+    return false;
 }
 
 bool UnfoldShortCircuitTraverser::visitAggregate(Visit visit, TIntermAggregate *node)