Change comma nodes to TIntermBinary

Comma nodes always have just two parameters. If there's an expression
with several commas in the middle, it's parsed as a tree of comma
operations. It makes more sense to represent it as a binary node
rather than an aggregate node.

After this patch, TIntermAggregate is still used for function
prototypes, function parameter lists, function calls, and variable and
invariant declarations.

BUG=angleproject:1490
TEST=angle_unittests, angle_end2end_tests

Change-Id: I66be10624bf27bcf25987b4d93958d4a07600771
Reviewed-on: https://chromium-review.googlesource.com/397320
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
diff --git a/src/compiler/translator/EmulatePrecision.cpp b/src/compiler/translator/EmulatePrecision.cpp
index 68789d6..a8f6d64 100644
--- a/src/compiler/translator/EmulatePrecision.cpp
+++ b/src/compiler/translator/EmulatePrecision.cpp
@@ -475,8 +475,8 @@
     {
         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;
     }
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 4ca8bea..0acfe40 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -823,6 +823,18 @@
     }
 }
 
+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 @@
     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());
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index 0c520a2..4c37034 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -462,6 +462,9 @@
 
     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;
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index bb75d9a..8e6519d 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -198,19 +198,11 @@
     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 @@
     }
     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;
 }
diff --git a/src/compiler/translator/Intermediate.h b/src/compiler/translator/Intermediate.h
index f5878bd..a4fb7cc 100644
--- a/src/compiler/translator/Intermediate.h
+++ b/src/compiler/translator/Intermediate.h
@@ -48,10 +48,10 @@
                              const TSourceLoc &line);
     TIntermCase *addCase(
         TIntermTyped *condition, const TSourceLoc &line);
-    TIntermTyped *addComma(TIntermTyped *left,
-                           TIntermTyped *right,
-                           const TSourceLoc &line,
-                           int shaderVersion);
+    static TIntermTyped *AddComma(TIntermTyped *left,
+                                  TIntermTyped *right,
+                                  const TSourceLoc &line,
+                                  int shaderVersion);
     TIntermConstantUnion *addConstantUnion(const TConstantUnion *constantUnion,
                                            const TType &type,
                                            const TSourceLoc &line);
diff --git a/src/compiler/translator/OutputGLSLBase.cpp b/src/compiler/translator/OutputGLSLBase.cpp
index 45b0af3..4301559 100644
--- a/src/compiler/translator/OutputGLSLBase.cpp
+++ b/src/compiler/translator/OutputGLSLBase.cpp
@@ -300,201 +300,205 @@
     TInfoSinkBase &out = objSink();
     switch (node->getOp())
     {
-      case EOpInitialize:
-        if (visit == InVisit)
-        {
-            out << " = ";
-            // RHS of initialize is not being declared.
-            mDeclaringVariables = false;
-        }
-        break;
-      case EOpAssign:
-        writeTriplet(visit, "(", " = ", ")");
-        break;
-      case EOpAddAssign:
-        writeTriplet(visit, "(", " += ", ")");
-        break;
-      case EOpSubAssign:
-        writeTriplet(visit, "(", " -= ", ")");
-        break;
-      case EOpDivAssign:
-        writeTriplet(visit, "(", " /= ", ")");
-        break;
-      case EOpIModAssign:
-        writeTriplet(visit, "(", " %= ", ")");
-        break;
-      // Notice the fall-through.
-      case EOpMulAssign:
-      case EOpVectorTimesMatrixAssign:
-      case EOpVectorTimesScalarAssign:
-      case EOpMatrixTimesScalarAssign:
-      case EOpMatrixTimesMatrixAssign:
-        writeTriplet(visit, "(", " *= ", ")");
-        break;
-      case EOpBitShiftLeftAssign:
-        writeTriplet(visit, "(", " <<= ", ")");
-        break;
-      case EOpBitShiftRightAssign:
-        writeTriplet(visit, "(", " >>= ", ")");
-        break;
-      case EOpBitwiseAndAssign:
-        writeTriplet(visit, "(", " &= ", ")");
-        break;
-      case EOpBitwiseXorAssign:
-        writeTriplet(visit, "(", " ^= ", ")");
-        break;
-      case EOpBitwiseOrAssign:
-        writeTriplet(visit, "(", " |= ", ")");
-        break;
-
-      case EOpIndexDirect:
-        writeTriplet(visit, NULL, "[", "]");
-        break;
-      case EOpIndexIndirect:
-        if (node->getAddIndexClamp())
-        {
+        case EOpComma:
+            writeTriplet(visit, "(", ", ", ")");
+            break;
+        case EOpInitialize:
             if (visit == InVisit)
             {
-                if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
-                    out << "[int(clamp(float(";
-                else
-                    out << "[webgl_int_clamp(";
+                out << " = ";
+                // RHS of initialize is not being declared.
+                mDeclaringVariables = false;
             }
-            else if (visit == PostVisit)
-            {
-                int maxSize;
-                TIntermTyped *left = node->getLeft();
-                TType leftType = left->getType();
+            break;
+        case EOpAssign:
+            writeTriplet(visit, "(", " = ", ")");
+            break;
+        case EOpAddAssign:
+            writeTriplet(visit, "(", " += ", ")");
+            break;
+        case EOpSubAssign:
+            writeTriplet(visit, "(", " -= ", ")");
+            break;
+        case EOpDivAssign:
+            writeTriplet(visit, "(", " /= ", ")");
+            break;
+        case EOpIModAssign:
+            writeTriplet(visit, "(", " %= ", ")");
+            break;
+        // Notice the fall-through.
+        case EOpMulAssign:
+        case EOpVectorTimesMatrixAssign:
+        case EOpVectorTimesScalarAssign:
+        case EOpMatrixTimesScalarAssign:
+        case EOpMatrixTimesMatrixAssign:
+            writeTriplet(visit, "(", " *= ", ")");
+            break;
+        case EOpBitShiftLeftAssign:
+            writeTriplet(visit, "(", " <<= ", ")");
+            break;
+        case EOpBitShiftRightAssign:
+            writeTriplet(visit, "(", " >>= ", ")");
+            break;
+        case EOpBitwiseAndAssign:
+            writeTriplet(visit, "(", " &= ", ")");
+            break;
+        case EOpBitwiseXorAssign:
+            writeTriplet(visit, "(", " ^= ", ")");
+            break;
+        case EOpBitwiseOrAssign:
+            writeTriplet(visit, "(", " |= ", ")");
+            break;
 
-                if (left->isArray())
-                {
-                    // The shader will fail validation if the array length is not > 0.
-                    maxSize = static_cast<int>(leftType.getArraySize()) - 1;
-                }
-                else
-                {
-                    maxSize = leftType.getNominalSize() - 1;
-                }
-
-                if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
-                    out << "), 0.0, float(" << maxSize << ")))]";
-                else
-                    out << ", 0, " << maxSize << ")]";
-            }
-        }
-        else
-        {
+        case EOpIndexDirect:
             writeTriplet(visit, NULL, "[", "]");
-        }
-        break;
-      case EOpIndexDirectStruct:
-        if (visit == InVisit)
-        {
-            // Here we are writing out "foo.bar", where "foo" is struct
-            // and "bar" is field. In AST, it is represented as a binary
-            // node, where left child represents "foo" and right child "bar".
-            // The node itself represents ".". The struct field "bar" is
-            // actually stored as an index into TStructure::fields.
-            out << ".";
-            const TStructure *structure = node->getLeft()->getType().getStruct();
-            const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion();
-            const TField *field = structure->fields()[index->getIConst(0)];
+            break;
+        case EOpIndexIndirect:
+            if (node->getAddIndexClamp())
+            {
+                if (visit == InVisit)
+                {
+                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
+                        out << "[int(clamp(float(";
+                    else
+                        out << "[webgl_int_clamp(";
+                }
+                else if (visit == PostVisit)
+                {
+                    int maxSize;
+                    TIntermTyped *left = node->getLeft();
+                    TType leftType     = left->getType();
 
-            TString fieldName = field->name();
-            if (!mSymbolTable.findBuiltIn(structure->name(), mShaderVersion))
+                    if (left->isArray())
+                    {
+                        // The shader will fail validation if the array length is not > 0.
+                        maxSize = static_cast<int>(leftType.getArraySize()) - 1;
+                    }
+                    else
+                    {
+                        maxSize = leftType.getNominalSize() - 1;
+                    }
+
+                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
+                        out << "), 0.0, float(" << maxSize << ")))]";
+                    else
+                        out << ", 0, " << maxSize << ")]";
+                }
+            }
+            else
+            {
+                writeTriplet(visit, NULL, "[", "]");
+            }
+            break;
+        case EOpIndexDirectStruct:
+            if (visit == InVisit)
+            {
+                // Here we are writing out "foo.bar", where "foo" is struct
+                // and "bar" is field. In AST, it is represented as a binary
+                // node, where left child represents "foo" and right child "bar".
+                // The node itself represents ".". The struct field "bar" is
+                // actually stored as an index into TStructure::fields.
+                out << ".";
+                const TStructure *structure       = node->getLeft()->getType().getStruct();
+                const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion();
+                const TField *field               = structure->fields()[index->getIConst(0)];
+
+                TString fieldName = field->name();
+                if (!mSymbolTable.findBuiltIn(structure->name(), mShaderVersion))
+                    fieldName = hashName(fieldName);
+
+                out << fieldName;
+                visitChildren = false;
+            }
+            break;
+        case EOpIndexDirectInterfaceBlock:
+            if (visit == InVisit)
+            {
+                out << ".";
+                const TInterfaceBlock *interfaceBlock =
+                    node->getLeft()->getType().getInterfaceBlock();
+                const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion();
+                const TField *field               = interfaceBlock->fields()[index->getIConst(0)];
+
+                TString fieldName = field->name();
+                ASSERT(!mSymbolTable.findBuiltIn(interfaceBlock->name(), mShaderVersion));
                 fieldName = hashName(fieldName);
 
-            out << fieldName;
-            visitChildren = false;
-        }
-        break;
-      case EOpIndexDirectInterfaceBlock:
-          if (visit == InVisit)
-          {
-              out << ".";
-              const TInterfaceBlock *interfaceBlock = node->getLeft()->getType().getInterfaceBlock();
-              const TIntermConstantUnion *index = node->getRight()->getAsConstantUnion();
-              const TField *field = interfaceBlock->fields()[index->getIConst(0)];
+                out << fieldName;
+                visitChildren = false;
+            }
+            break;
 
-              TString fieldName = field->name();
-              ASSERT(!mSymbolTable.findBuiltIn(interfaceBlock->name(), mShaderVersion));
-              fieldName = hashName(fieldName);
+        case EOpAdd:
+            writeTriplet(visit, "(", " + ", ")");
+            break;
+        case EOpSub:
+            writeTriplet(visit, "(", " - ", ")");
+            break;
+        case EOpMul:
+            writeTriplet(visit, "(", " * ", ")");
+            break;
+        case EOpDiv:
+            writeTriplet(visit, "(", " / ", ")");
+            break;
+        case EOpIMod:
+            writeTriplet(visit, "(", " % ", ")");
+            break;
+        case EOpBitShiftLeft:
+            writeTriplet(visit, "(", " << ", ")");
+            break;
+        case EOpBitShiftRight:
+            writeTriplet(visit, "(", " >> ", ")");
+            break;
+        case EOpBitwiseAnd:
+            writeTriplet(visit, "(", " & ", ")");
+            break;
+        case EOpBitwiseXor:
+            writeTriplet(visit, "(", " ^ ", ")");
+            break;
+        case EOpBitwiseOr:
+            writeTriplet(visit, "(", " | ", ")");
+            break;
 
-              out << fieldName;
-              visitChildren = false;
-          }
-          break;
+        case EOpEqual:
+            writeTriplet(visit, "(", " == ", ")");
+            break;
+        case EOpNotEqual:
+            writeTriplet(visit, "(", " != ", ")");
+            break;
+        case EOpLessThan:
+            writeTriplet(visit, "(", " < ", ")");
+            break;
+        case EOpGreaterThan:
+            writeTriplet(visit, "(", " > ", ")");
+            break;
+        case EOpLessThanEqual:
+            writeTriplet(visit, "(", " <= ", ")");
+            break;
+        case EOpGreaterThanEqual:
+            writeTriplet(visit, "(", " >= ", ")");
+            break;
 
-      case EOpAdd:
-        writeTriplet(visit, "(", " + ", ")");
-        break;
-      case EOpSub:
-        writeTriplet(visit, "(", " - ", ")");
-        break;
-      case EOpMul:
-        writeTriplet(visit, "(", " * ", ")");
-        break;
-      case EOpDiv:
-        writeTriplet(visit, "(", " / ", ")");
-        break;
-      case EOpIMod:
-        writeTriplet(visit, "(", " % ", ")");
-        break;
-      case EOpBitShiftLeft:
-        writeTriplet(visit, "(", " << ", ")");
-        break;
-      case EOpBitShiftRight:
-        writeTriplet(visit, "(", " >> ", ")");
-        break;
-      case EOpBitwiseAnd:
-        writeTriplet(visit, "(", " & ", ")");
-        break;
-      case EOpBitwiseXor:
-        writeTriplet(visit, "(", " ^ ", ")");
-        break;
-      case EOpBitwiseOr:
-        writeTriplet(visit, "(", " | ", ")");
-        break;
+        // Notice the fall-through.
+        case EOpVectorTimesScalar:
+        case EOpVectorTimesMatrix:
+        case EOpMatrixTimesVector:
+        case EOpMatrixTimesScalar:
+        case EOpMatrixTimesMatrix:
+            writeTriplet(visit, "(", " * ", ")");
+            break;
 
-      case EOpEqual:
-        writeTriplet(visit, "(", " == ", ")");
-        break;
-      case EOpNotEqual:
-        writeTriplet(visit, "(", " != ", ")");
-        break;
-      case EOpLessThan:
-        writeTriplet(visit, "(", " < ", ")");
-        break;
-      case EOpGreaterThan:
-        writeTriplet(visit, "(", " > ", ")");
-        break;
-      case EOpLessThanEqual:
-        writeTriplet(visit, "(", " <= ", ")");
-        break;
-      case EOpGreaterThanEqual:
-        writeTriplet(visit, "(", " >= ", ")");
-        break;
-
-      // Notice the fall-through.
-      case EOpVectorTimesScalar:
-      case EOpVectorTimesMatrix:
-      case EOpMatrixTimesVector:
-      case EOpMatrixTimesScalar:
-      case EOpMatrixTimesMatrix:
-        writeTriplet(visit, "(", " * ", ")");
-        break;
-
-      case EOpLogicalOr:
-        writeTriplet(visit, "(", " || ", ")");
-        break;
-      case EOpLogicalXor:
-        writeTriplet(visit, "(", " ^^ ", ")");
-        break;
-      case EOpLogicalAnd:
-        writeTriplet(visit, "(", " && ", ")");
-        break;
-      default:
-        UNREACHABLE();
+        case EOpLogicalOr:
+            writeTriplet(visit, "(", " || ", ")");
+            break;
+        case EOpLogicalXor:
+            writeTriplet(visit, "(", " ^^ ", ")");
+            break;
+        case EOpLogicalAnd:
+            writeTriplet(visit, "(", " && ", ")");
+            break;
+        default:
+            UNREACHABLE();
     }
 
     return visitChildren;
@@ -941,9 +945,6 @@
       case EOpVectorNotEqual:
         writeBuiltInFunctionTriplet(visit, "notEqual(", useEmulatedFunction);
         break;
-      case EOpComma:
-        writeTriplet(visit, "(", ", ", ")");
-        break;
 
       case EOpMod:
         writeBuiltInFunctionTriplet(visit, "mod(", useEmulatedFunction);
diff --git a/src/compiler/translator/OutputHLSL.cpp b/src/compiler/translator/OutputHLSL.cpp
index d4ba304..feae684 100644
--- a/src/compiler/translator/OutputHLSL.cpp
+++ b/src/compiler/translator/OutputHLSL.cpp
@@ -867,133 +867,139 @@
 
     switch (node->getOp())
     {
-      case EOpAssign:
-        if (node->getLeft()->isArray())
-        {
-            TIntermAggregate *rightAgg = node->getRight()->getAsAggregate();
-            if (rightAgg != nullptr && rightAgg->isConstructor())
+        case EOpComma:
+            outputTriplet(out, visit, "(", ", ", ")");
+            break;
+        case EOpAssign:
+            if (node->getLeft()->isArray())
             {
-                const TString &functionName = addArrayConstructIntoFunction(node->getType());
-                out << functionName << "(";
-                node->getLeft()->traverse(this);
-                TIntermSequence *seq = rightAgg->getSequence();
-                for (auto &arrayElement : *seq)
+                TIntermAggregate *rightAgg = node->getRight()->getAsAggregate();
+                if (rightAgg != nullptr && rightAgg->isConstructor())
                 {
-                    out << ", ";
-                    arrayElement->traverse(this);
+                    const TString &functionName = addArrayConstructIntoFunction(node->getType());
+                    out << functionName << "(";
+                    node->getLeft()->traverse(this);
+                    TIntermSequence *seq = rightAgg->getSequence();
+                    for (auto &arrayElement : *seq)
+                    {
+                        out << ", ";
+                        arrayElement->traverse(this);
+                    }
+                    out << ")";
+                    return false;
                 }
-                out << ")";
-                return false;
+                // ArrayReturnValueToOutParameter should have eliminated expressions where a
+                // function call is assigned.
+                ASSERT(rightAgg == nullptr || rightAgg->getOp() != EOpFunctionCall);
+
+                const TString &functionName = addArrayAssignmentFunction(node->getType());
+                outputTriplet(out, visit, (functionName + "(").c_str(), ", ", ")");
             }
-            // ArrayReturnValueToOutParameter should have eliminated expressions where a function call is assigned.
-            ASSERT(rightAgg == nullptr || rightAgg->getOp() != EOpFunctionCall);
-
-            const TString &functionName = addArrayAssignmentFunction(node->getType());
-            outputTriplet(out, visit, (functionName + "(").c_str(), ", ", ")");
-        }
-        else
-        {
-            outputTriplet(out, visit, "(", " = ", ")");
-        }
-        break;
-      case EOpInitialize:
-        if (visit == PreVisit)
-        {
-            TIntermSymbol *symbolNode = node->getLeft()->getAsSymbolNode();
-            ASSERT(symbolNode);
-            TIntermTyped *expression = node->getRight();
-
-            // Global initializers must be constant at this point.
-            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
-            // this to "float t = x, x = t;".
-            if (writeSameSymbolInitializer(out, symbolNode, expression))
+            else
             {
-                // Skip initializing the rest of the expression
-                return false;
+                outputTriplet(out, visit, "(", " = ", ")");
             }
-            else if (writeConstantInitialization(out, symbolNode, expression))
+            break;
+        case EOpInitialize:
+            if (visit == PreVisit)
             {
-                return false;
+                TIntermSymbol *symbolNode = node->getLeft()->getAsSymbolNode();
+                ASSERT(symbolNode);
+                TIntermTyped *expression = node->getRight();
+
+                // Global initializers must be constant at this point.
+                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
+                // this to "float t = x, x = t;".
+                if (writeSameSymbolInitializer(out, symbolNode, expression))
+                {
+                    // Skip initializing the rest of the expression
+                    return false;
+                }
+                else if (writeConstantInitialization(out, symbolNode, expression))
+                {
+                    return false;
+                }
             }
-        }
-        else if (visit == InVisit)
-        {
-            out << " = ";
-        }
-        break;
-      case EOpAddAssign:
-          outputTriplet(out, visit, "(", " += ", ")");
-          break;
-      case EOpSubAssign:
-          outputTriplet(out, visit, "(", " -= ", ")");
-          break;
-      case EOpMulAssign:
-          outputTriplet(out, visit, "(", " *= ", ")");
-          break;
-      case EOpVectorTimesScalarAssign:
-          outputTriplet(out, visit, "(", " *= ", ")");
-          break;
-      case EOpMatrixTimesScalarAssign:
-          outputTriplet(out, visit, "(", " *= ", ")");
-          break;
-      case EOpVectorTimesMatrixAssign:
-        if (visit == PreVisit)
-        {
-            out << "(";
-        }
-        else if (visit == InVisit)
-        {
-            out << " = mul(";
-            node->getLeft()->traverse(this);
-            out << ", transpose(";
-        }
-        else
-        {
-            out << ")))";
-        }
-        break;
-      case EOpMatrixTimesMatrixAssign:
-        if (visit == PreVisit)
-        {
-            out << "(";
-        }
-        else if (visit == InVisit)
-        {
-            out << " = transpose(mul(transpose(";
-            node->getLeft()->traverse(this);
-            out << "), transpose(";
-        }
-        else
-        {
-            out << "))))";
-        }
-        break;
-      case EOpDivAssign:
-          outputTriplet(out, visit, "(", " /= ", ")");
-          break;
-      case EOpIModAssign:
-          outputTriplet(out, visit, "(", " %= ", ")");
-          break;
-      case EOpBitShiftLeftAssign:
-          outputTriplet(out, visit, "(", " <<= ", ")");
-          break;
-      case EOpBitShiftRightAssign:
-          outputTriplet(out, visit, "(", " >>= ", ")");
-          break;
-      case EOpBitwiseAndAssign:
-          outputTriplet(out, visit, "(", " &= ", ")");
-          break;
-      case EOpBitwiseXorAssign:
-          outputTriplet(out, visit, "(", " ^= ", ")");
-          break;
-      case EOpBitwiseOrAssign:
-          outputTriplet(out, visit, "(", " |= ", ")");
-          break;
-      case EOpIndexDirect:
+            else if (visit == InVisit)
+            {
+                out << " = ";
+            }
+            break;
+        case EOpAddAssign:
+            outputTriplet(out, visit, "(", " += ", ")");
+            break;
+        case EOpSubAssign:
+            outputTriplet(out, visit, "(", " -= ", ")");
+            break;
+        case EOpMulAssign:
+            outputTriplet(out, visit, "(", " *= ", ")");
+            break;
+        case EOpVectorTimesScalarAssign:
+            outputTriplet(out, visit, "(", " *= ", ")");
+            break;
+        case EOpMatrixTimesScalarAssign:
+            outputTriplet(out, visit, "(", " *= ", ")");
+            break;
+        case EOpVectorTimesMatrixAssign:
+            if (visit == PreVisit)
+            {
+                out << "(";
+            }
+            else if (visit == InVisit)
+            {
+                out << " = mul(";
+                node->getLeft()->traverse(this);
+                out << ", transpose(";
+            }
+            else
+            {
+                out << ")))";
+            }
+            break;
+        case EOpMatrixTimesMatrixAssign:
+            if (visit == PreVisit)
+            {
+                out << "(";
+            }
+            else if (visit == InVisit)
+            {
+                out << " = transpose(mul(transpose(";
+                node->getLeft()->traverse(this);
+                out << "), transpose(";
+            }
+            else
+            {
+                out << "))))";
+            }
+            break;
+        case EOpDivAssign:
+            outputTriplet(out, visit, "(", " /= ", ")");
+            break;
+        case EOpIModAssign:
+            outputTriplet(out, visit, "(", " %= ", ")");
+            break;
+        case EOpBitShiftLeftAssign:
+            outputTriplet(out, visit, "(", " <<= ", ")");
+            break;
+        case EOpBitShiftRightAssign:
+            outputTriplet(out, visit, "(", " >>= ", ")");
+            break;
+        case EOpBitwiseAndAssign:
+            outputTriplet(out, visit, "(", " &= ", ")");
+            break;
+        case EOpBitwiseXorAssign:
+            outputTriplet(out, visit, "(", " ^= ", ")");
+            break;
+        case EOpBitwiseOrAssign:
+            outputTriplet(out, visit, "(", " |= ", ")");
+            break;
+        case EOpIndexDirect:
         {
             const TType& leftType = node->getLeft()->getType();
             if (leftType.isInterfaceBlock())
@@ -1642,9 +1648,6 @@
                 return false;
             }
             break;
-        case EOpComma:
-            outputTriplet(out, visit, "(", ", ", ")");
-            break;
         case EOpFunctionCall:
         {
             TIntermSequence *arguments = node->getSequence();
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 488863e..79b2b06 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3630,7 +3630,7 @@
               ",");
     }
 
-    return intermediate.addComma(left, right, loc, mShaderVersion);
+    return TIntermediate::AddComma(left, right, loc, mShaderVersion);
 }
 
 TIntermBranch *TParseContext::addBranch(TOperator op, const TSourceLoc &loc)
diff --git a/src/compiler/translator/SplitSequenceOperator.cpp b/src/compiler/translator/SplitSequenceOperator.cpp
index ff6443d..e0d28b9 100644
--- a/src/compiler/translator/SplitSequenceOperator.cpp
+++ b/src/compiler/translator/SplitSequenceOperator.cpp
@@ -57,7 +57,7 @@
     nextTemporaryIndex();
 }
 
-bool SplitSequenceOperatorTraverser::visitBinary(Visit visit, TIntermBinary *node)
+bool SplitSequenceOperatorTraverser::visitAggregate(Visit visit, TIntermAggregate *node)
 {
     if (mFoundExpressionToSplit)
         return false;
@@ -65,15 +65,14 @@
     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 @@
             // 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 @@
     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;
     }
 
diff --git a/src/compiler/translator/intermOut.cpp b/src/compiler/translator/intermOut.cpp
index e792925..352fde3 100644
--- a/src/compiler/translator/intermOut.cpp
+++ b/src/compiler/translator/intermOut.cpp
@@ -105,145 +105,148 @@
 
     switch (node->getOp())
     {
-      case EOpAssign:
-        out << "move second child to first child";
-        break;
-      case EOpInitialize:
-        out << "initialize first child with second child";
-        break;
-      case EOpAddAssign:
-        out << "add second child into first child";
-        break;
-      case EOpSubAssign:
-        out << "subtract second child into first child";
-        break;
-      case EOpMulAssign:
-        out << "multiply second child into first child";
-        break;
-      case EOpVectorTimesMatrixAssign:
-        out << "matrix mult second child into first child";
-        break;
-      case EOpVectorTimesScalarAssign:
-        out << "vector scale second child into first child";
-        break;
-      case EOpMatrixTimesScalarAssign:
-        out << "matrix scale second child into first child";
-        break;
-      case EOpMatrixTimesMatrixAssign:
-        out << "matrix mult second child into first child";
-        break;
-      case EOpDivAssign:
-        out << "divide second child into first child";
-        break;
-      case EOpIModAssign:
-        out << "modulo second child into first child";
-        break;
-      case EOpBitShiftLeftAssign:
-        out << "bit-wise shift first child left by second child";
-        break;
-      case EOpBitShiftRightAssign:
-        out << "bit-wise shift first child right by second child";
-        break;
-      case EOpBitwiseAndAssign:
-        out << "bit-wise and second child into first child";
-        break;
-      case EOpBitwiseXorAssign:
-        out << "bit-wise xor second child into first child";
-        break;
-      case EOpBitwiseOrAssign:
-        out << "bit-wise or second child into first child";
-        break;
+        case EOpComma:
+            out << "comma";
+            break;
+        case EOpAssign:
+            out << "move second child to first child";
+            break;
+        case EOpInitialize:
+            out << "initialize first child with second child";
+            break;
+        case EOpAddAssign:
+            out << "add second child into first child";
+            break;
+        case EOpSubAssign:
+            out << "subtract second child into first child";
+            break;
+        case EOpMulAssign:
+            out << "multiply second child into first child";
+            break;
+        case EOpVectorTimesMatrixAssign:
+            out << "matrix mult second child into first child";
+            break;
+        case EOpVectorTimesScalarAssign:
+            out << "vector scale second child into first child";
+            break;
+        case EOpMatrixTimesScalarAssign:
+            out << "matrix scale second child into first child";
+            break;
+        case EOpMatrixTimesMatrixAssign:
+            out << "matrix mult second child into first child";
+            break;
+        case EOpDivAssign:
+            out << "divide second child into first child";
+            break;
+        case EOpIModAssign:
+            out << "modulo second child into first child";
+            break;
+        case EOpBitShiftLeftAssign:
+            out << "bit-wise shift first child left by second child";
+            break;
+        case EOpBitShiftRightAssign:
+            out << "bit-wise shift first child right by second child";
+            break;
+        case EOpBitwiseAndAssign:
+            out << "bit-wise and second child into first child";
+            break;
+        case EOpBitwiseXorAssign:
+            out << "bit-wise xor second child into first child";
+            break;
+        case EOpBitwiseOrAssign:
+            out << "bit-wise or second child into first child";
+            break;
 
-      case EOpIndexDirect:
-        out << "direct index";
-        break;
-      case EOpIndexIndirect:
-        out << "indirect index";
-        break;
-      case EOpIndexDirectStruct:
-        out << "direct index for structure";
-        break;
-      case EOpIndexDirectInterfaceBlock:
-        out << "direct index for interface block";
-        break;
+        case EOpIndexDirect:
+            out << "direct index";
+            break;
+        case EOpIndexIndirect:
+            out << "indirect index";
+            break;
+        case EOpIndexDirectStruct:
+            out << "direct index for structure";
+            break;
+        case EOpIndexDirectInterfaceBlock:
+            out << "direct index for interface block";
+            break;
 
-      case EOpAdd:
-        out << "add";
-        break;
-      case EOpSub:
-        out << "subtract";
-        break;
-      case EOpMul:
-        out << "component-wise multiply";
-        break;
-      case EOpDiv:
-        out << "divide";
-        break;
-      case EOpIMod:
-        out << "modulo";
-        break;
-      case EOpBitShiftLeft:
-        out << "bit-wise shift left";
-        break;
-      case EOpBitShiftRight:
-        out << "bit-wise shift right";
-        break;
-      case EOpBitwiseAnd:
-        out << "bit-wise and";
-        break;
-      case EOpBitwiseXor:
-        out << "bit-wise xor";
-        break;
-      case EOpBitwiseOr:
-        out << "bit-wise or";
-        break;
+        case EOpAdd:
+            out << "add";
+            break;
+        case EOpSub:
+            out << "subtract";
+            break;
+        case EOpMul:
+            out << "component-wise multiply";
+            break;
+        case EOpDiv:
+            out << "divide";
+            break;
+        case EOpIMod:
+            out << "modulo";
+            break;
+        case EOpBitShiftLeft:
+            out << "bit-wise shift left";
+            break;
+        case EOpBitShiftRight:
+            out << "bit-wise shift right";
+            break;
+        case EOpBitwiseAnd:
+            out << "bit-wise and";
+            break;
+        case EOpBitwiseXor:
+            out << "bit-wise xor";
+            break;
+        case EOpBitwiseOr:
+            out << "bit-wise or";
+            break;
 
-      case EOpEqual:
-        out << "Compare Equal";
-        break;
-      case EOpNotEqual:
-        out << "Compare Not Equal";
-        break;
-      case EOpLessThan:
-        out << "Compare Less Than";
-        break;
-      case EOpGreaterThan:
-        out << "Compare Greater Than";
-        break;
-      case EOpLessThanEqual:
-        out << "Compare Less Than or Equal";
-        break;
-      case EOpGreaterThanEqual:
-        out << "Compare Greater Than or Equal";
-        break;
+        case EOpEqual:
+            out << "Compare Equal";
+            break;
+        case EOpNotEqual:
+            out << "Compare Not Equal";
+            break;
+        case EOpLessThan:
+            out << "Compare Less Than";
+            break;
+        case EOpGreaterThan:
+            out << "Compare Greater Than";
+            break;
+        case EOpLessThanEqual:
+            out << "Compare Less Than or Equal";
+            break;
+        case EOpGreaterThanEqual:
+            out << "Compare Greater Than or Equal";
+            break;
 
-      case EOpVectorTimesScalar:
-        out << "vector-scale";
-        break;
-      case EOpVectorTimesMatrix:
-        out << "vector-times-matrix";
-        break;
-      case EOpMatrixTimesVector:
-        out << "matrix-times-vector";
-        break;
-      case EOpMatrixTimesScalar:
-        out << "matrix-scale";
-        break;
-      case EOpMatrixTimesMatrix:
-        out << "matrix-multiply";
-        break;
+        case EOpVectorTimesScalar:
+            out << "vector-scale";
+            break;
+        case EOpVectorTimesMatrix:
+            out << "vector-times-matrix";
+            break;
+        case EOpMatrixTimesVector:
+            out << "matrix-times-vector";
+            break;
+        case EOpMatrixTimesScalar:
+            out << "matrix-scale";
+            break;
+        case EOpMatrixTimesMatrix:
+            out << "matrix-multiply";
+            break;
 
-      case EOpLogicalOr:
-        out << "logical-or";
-        break;
-      case EOpLogicalXor:
-        out << "logical-xor";
-        break;
-      case EOpLogicalAnd:
-        out << "logical-and";
-        break;
-      default:
-        out << "<unknown op>";
+        case EOpLogicalOr:
+            out << "logical-or";
+            break;
+        case EOpLogicalXor:
+            out << "logical-xor";
+            break;
+        case EOpLogicalAnd:
+            out << "logical-and";
+            break;
+        default:
+            out << "<unknown op>";
     }
 
     out << " (" << node->getCompleteString() << ")";
@@ -399,7 +402,6 @@
 
     switch (node->getOp())
     {
-      case EOpComma:         out << "Comma\n"; return true;
       case EOpFunctionCall:
           OutputFunction(out, "Function Call", node->getFunctionSymbolInfo());
           break;