Change comma nodes to TIntermBinary

src/compiler/translator/EmulatePrecision.cpp

@@ -475,8 +475,8 @@ bool parentUsesResult(TIntermNode* parent, TIntermNode* node)
     {
         return false;
     }
-    TIntermAggregate *aggParent = parent->getAsAggregate();
-    if (aggParent && aggParent->getOp() == EOpComma && (aggParent->getSequence()->back() != node))
+    TIntermBinary *binaryParent = parent->getAsBinaryNode();
+    if (binaryParent && binaryParent->getOp() == EOpComma && (binaryParent->getRight() != node))
     {
         return false;
     }

src/compiler/translator/IntermNode.cpp

@@ -823,6 +823,18 @@ void TIntermSwizzle::writeOffsetsAsXYZW(TInfoSinkBase *out) const
     }
 }
 
+TQualifier TIntermBinary::GetCommaQualifier(int shaderVersion,
+                                            const TIntermTyped *left,
+                                            const TIntermTyped *right)
+{
+    // ESSL3.00 section 12.43: The result of a sequence operator is not a constant-expression.
+    if (shaderVersion >= 300 || left->getQualifier() != EvqConst ||
+        right->getQualifier() != EvqConst)
+    {
+        return EvqTemporary;
+    }
+    return EvqConst;
+}
 
 // Establishes the type of the result of the binary operation.
 void TIntermBinary::promote()
@@ -830,6 +842,13 @@ void TIntermBinary::promote()
     ASSERT(!isMultiplication() ||
            mOp == GetMulOpBasedOnOperands(mLeft->getType(), mRight->getType()));
 
+    // Comma is handled as a special case.
+    if (mOp == EOpComma)
+    {
+        setType(mRight->getType());
+        return;
+    }
+
     // Base assumption:  just make the type the same as the left
     // operand.  Then only deviations from this need be coded.
     setType(mLeft->getType());

src/compiler/translator/IntermNode.h

@@ -462,6 +462,9 @@ class TIntermBinary : public TIntermOperator
 
     static TOperator GetMulOpBasedOnOperands(const TType &left, const TType &right);
     static TOperator GetMulAssignOpBasedOnOperands(const TType &left, const TType &right);
+    static TQualifier GetCommaQualifier(int shaderVersion,
+                                        const TIntermTyped *left,
+                                        const TIntermTyped *right);
 
     TIntermBinary *getAsBinaryNode() override { return this; };
     void traverse(TIntermTraverser *it) override;

src/compiler/translator/Intermediate.cpp

@@ -198,19 +198,11 @@ TIntermNode *TIntermediate::addIfElse(TIntermTyped *cond,
     return node;
 }
 
-TIntermTyped *TIntermediate::addComma(TIntermTyped *left,
+TIntermTyped *TIntermediate::AddComma(TIntermTyped *left,
                                       TIntermTyped *right,
                                       const TSourceLoc &line,
                                       int shaderVersion)
 {
-    TQualifier resultQualifier = EvqConst;
-    // ESSL3.00 section 12.43: The result of a sequence operator is not a constant-expression.
-    if (shaderVersion >= 300 || left->getQualifier() != EvqConst ||
-        right->getQualifier() != EvqConst)
-    {
-        resultQualifier = EvqTemporary;
-    }
-
     TIntermTyped *commaNode = nullptr;
     if (!left->hasSideEffects())
     {
@@ -218,10 +210,10 @@ TIntermTyped *TIntermediate::addComma(TIntermTyped *left,
     }
     else
     {
-        commaNode = growAggregate(left, right, line);
-        commaNode->getAsAggregate()->setOp(EOpComma);
-        commaNode->setType(right->getType());
+        commaNode = new TIntermBinary(EOpComma, left, right);
+        commaNode->setLine(line);
     }
+    TQualifier resultQualifier = TIntermBinary::GetCommaQualifier(shaderVersion, left, right);
     commaNode->getTypePointer()->setQualifier(resultQualifier);
     return commaNode;
 }

src/compiler/translator/Intermediate.h

@@ -48,7 +48,7 @@ class TIntermediate
                              const TSourceLoc &line);
     TIntermCase *addCase(
         TIntermTyped *condition, const TSourceLoc &line);
-    TIntermTyped *addComma(TIntermTyped *left,
+    static TIntermTyped *AddComma(TIntermTyped *left,
                                   TIntermTyped *right,
                                   const TSourceLoc &line,
                                   int shaderVersion);

src/compiler/translator/OutputGLSLBase.cpp

@@ -300,6 +300,9 @@ bool TOutputGLSLBase::visitBinary(Visit visit, TIntermBinary *node)
     TInfoSinkBase &out = objSink();
     switch (node->getOp())
     {
+        case EOpComma:
+            writeTriplet(visit, "(", ", ", ")");
+            break;
         case EOpInitialize:
             if (visit == InVisit)
             {
@@ -412,7 +415,8 @@ bool TOutputGLSLBase::visitBinary(Visit visit, TIntermBinary *node)
             if (visit == InVisit)
             {
                 out << ".";
-              const TInterfaceBlock *interfaceBlock = node->getLeft()->getType().getInterfaceBlock();
+                const TInterfaceBlock *interfaceBlock =
+                    node->getLeft()->getType().getInterfaceBlock();
                 const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion();
                 const TField *field               = interfaceBlock->fields()[index->getIConst(0)];
 
@@ -941,9 +945,6 @@ bool TOutputGLSLBase::visitAggregate(Visit visit, TIntermAggregate *node)
       case EOpVectorNotEqual:
         writeBuiltInFunctionTriplet(visit, "notEqual(", useEmulatedFunction);
         break;
-      case EOpComma:
-        writeTriplet(visit, "(", ", ", ")");
-        break;
 
       case EOpMod:
         writeBuiltInFunctionTriplet(visit, "mod(", useEmulatedFunction);

src/compiler/translator/OutputHLSL.cpp

@@ -867,6 +867,9 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
 
     switch (node->getOp())
     {
+        case EOpComma:
+            outputTriplet(out, visit, "(", ", ", ")");
+            break;
         case EOpAssign:
             if (node->getLeft()->isArray())
             {
@@ -885,7 +888,8 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
                     out << ")";
                     return false;
                 }
-            // ArrayReturnValueToOutParameter should have eliminated expressions where a function call is assigned.
+                // ArrayReturnValueToOutParameter should have eliminated expressions where a
+                // function call is assigned.
                 ASSERT(rightAgg == nullptr || rightAgg->getOp() != EOpFunctionCall);
 
                 const TString &functionName = addArrayAssignmentFunction(node->getType());
@@ -904,11 +908,13 @@ bool OutputHLSL::visitBinary(Visit visit, TIntermBinary *node)
                 TIntermTyped *expression = node->getRight();
 
                 // Global initializers must be constant at this point.
-            ASSERT(symbolNode->getQualifier() != EvqGlobal || canWriteAsHLSLLiteral(expression));
+                ASSERT(symbolNode->getQualifier() != EvqGlobal ||
+                       canWriteAsHLSLLiteral(expression));
 
                 // GLSL allows to write things like "float x = x;" where a new variable x is defined
                 // and the value of an existing variable x is assigned. HLSL uses C semantics (the
-            // new variable is created before the assignment is evaluated), so we need to convert
+                // new variable is created before the assignment is evaluated), so we need to
+                // convert
                 // this to "float t = x, x = t;".
                 if (writeSameSymbolInitializer(out, symbolNode, expression))
                 {
@@ -1642,9 +1648,6 @@ bool OutputHLSL::visitAggregate(Visit visit, TIntermAggregate *node)
                 return false;
             }
             break;
-        case EOpComma:
-            outputTriplet(out, visit, "(", ", ", ")");
-            break;
         case EOpFunctionCall:
         {
             TIntermSequence *arguments = node->getSequence();

src/compiler/translator/ParseContext.cpp

@@ -3630,7 +3630,7 @@ TIntermTyped *TParseContext::addComma(TIntermTyped *left,
               ",");
     }
 
-    return intermediate.addComma(left, right, loc, mShaderVersion);
+    return TIntermediate::AddComma(left, right, loc, mShaderVersion);
 }
 
 TIntermBranch *TParseContext::addBranch(TOperator op, const TSourceLoc &loc)

src/compiler/translator/SplitSequenceOperator.cpp

@@ -57,7 +57,7 @@ void SplitSequenceOperatorTraverser::nextIteration()
     nextTemporaryIndex();
 }
 
-bool SplitSequenceOperatorTraverser::visitBinary(Visit visit, TIntermBinary *node)
+bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 {
     if (mFoundExpressionToSplit)
         return false;
@@ -65,15 +65,14 @@ bool SplitSequenceOperatorTraverser::visitBinary(Visit visit, TIntermBinary *nod
     if (mInsideSequenceOperator > 0 && visit == PreVisit)
     {
         // Detect expressions that need to be simplified
-        mFoundExpressionToSplit =
-            mPatternToSplitMatcher.match(node, getParentNode(), isLValueRequiredHere());
+        mFoundExpressionToSplit = mPatternToSplitMatcher.match(node, getParentNode());
         return !mFoundExpressionToSplit;
     }
 
     return true;
 }
 
-bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
+bool SplitSequenceOperatorTraverser::visitBinary(Visit visit, TIntermBinary *node)
 {
     if (node->getOp() == EOpComma)
     {
@@ -91,19 +90,12 @@ bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregat
             // execution order.
             if (mFoundExpressionToSplit && mInsideSequenceOperator == 1)
             {
-                // Move all operands of the sequence operation except the last one into separate
-                // statements in the parent block.
+                // Move the left side operand into a separate statement in the parent block.
                 TIntermSequence insertions;
-                for (auto *sequenceChild : *node->getSequence())
-                {
-                    if (sequenceChild != node->getSequence()->back())
-                    {
-                        insertions.push_back(sequenceChild);
-                    }
-                }
+                insertions.push_back(node->getLeft());
                 insertStatementsInParentBlock(insertions);
-                // Replace the sequence with its last operand
-                queueReplacement(node, node->getSequence()->back(), OriginalNode::IS_DROPPED);
+                // Replace the comma node with its right side operand.
+                queueReplacement(node, node->getRight(), OriginalNode::IS_DROPPED);
             }
             mInsideSequenceOperator--;
         }
@@ -116,7 +108,8 @@ bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregat
     if (mInsideSequenceOperator > 0 && visit == PreVisit)
     {
         // Detect expressions that need to be simplified
-        mFoundExpressionToSplit = mPatternToSplitMatcher.match(node, getParentNode());
+        mFoundExpressionToSplit =
+            mPatternToSplitMatcher.match(node, getParentNode(), isLValueRequiredHere());
         return !mFoundExpressionToSplit;
     }
 

src/compiler/translator/intermOut.cpp

@@ -105,6 +105,9 @@ bool TOutputTraverser::visitBinary(Visit visit, TIntermBinary *node)
 
     switch (node->getOp())
     {
+        case EOpComma:
+            out << "comma";
+            break;
         case EOpAssign:
             out << "move second child to first child";
             break;
@@ -399,7 +402,6 @@ bool TOutputTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 
     switch (node->getOp())
     {
-      case EOpComma:         out << "Comma\n"; return true;
       case EOpFunctionCall:
           OutputFunction(out, "Function Call", node->getFunctionSymbolInfo());
           break;