commit 1dded803d767f24247e47bbb85f9d83e88bceee3
author Olli Etuaho <oetuaho@nvidia.com> Thu Aug 18 18:13:13 2016 +0300
committer Commit Bot <commit-bot@chromium.org> Thu Aug 25 19:39:12 2016 +0000
tree 4c5b9fa97ef37b973d5fd15cb39f4771f61ffa25
parent 9949f04d6b5756c04e0cf441ea0c78d05be42c9c

Check multiplication validity in ParseContext

This improves separation of responsibilities in the code: ParseContext
should handle operand type validation, while TIntermBinary::promote
should ideally only determine the type of the node based on the
operation and operands.

BUG=angleproject:952
TEST=angle_unittests

Change-Id: I9a8d8ede21cdf35de631623a62194c0da5c604d2
Reviewed-on: https://chromium-review.googlesource.com/372622
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>

src/compiler/translator/ParseContext.cpp

diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index eac0702..abe3712 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3574,6 +3574,49 @@
     return true;
 }
 
+bool TParseContext::isMultiplicationTypeCombinationValid(TOperator op,
+                                                         const TType &left,
+                                                         const TType &right)
+{
+    switch (op)
+    {
+        case EOpMul:
+        case EOpMulAssign:
+            return left.getNominalSize() == right.getNominalSize() &&
+                   left.getSecondarySize() == right.getSecondarySize();
+        case EOpVectorTimesScalar:
+            return true;
+        case EOpVectorTimesScalarAssign:
+            ASSERT(!left.isMatrix() && !right.isMatrix());
+            return left.isVector() && !right.isVector();
+        case EOpVectorTimesMatrix:
+            return left.getNominalSize() == right.getRows();
+        case EOpVectorTimesMatrixAssign:
+            ASSERT(!left.isMatrix() && right.isMatrix());
+            return left.isVector() && left.getNominalSize() == right.getRows() &&
+                   left.getNominalSize() == right.getCols();
+        case EOpMatrixTimesVector:
+            return left.getCols() == right.getNominalSize();
+        case EOpMatrixTimesScalar:
+            return true;
+        case EOpMatrixTimesScalarAssign:
+            ASSERT(left.isMatrix() && !right.isMatrix());
+            return !right.isVector();
+        case EOpMatrixTimesMatrix:
+            return left.getCols() == right.getRows();
+        case EOpMatrixTimesMatrixAssign:
+            ASSERT(left.isMatrix() && right.isMatrix());
+            // We need to check two things:
+            // 1. The matrix multiplication step is valid.
+            // 2. The result will have the same number of columns as the lvalue.
+            return left.getCols() == right.getRows() && left.getCols() == right.getCols();
+
+        default:
+            UNREACHABLE();
+            return false;
+    }
+}
+
 TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op,
                                                    TIntermTyped *left,
                                                    TIntermTyped *right,
@@ -3634,6 +3677,15 @@
             break;
     }
 
+    if (op == EOpMul)
+    {
+        op = TIntermBinary::GetMulOpBasedOnOperands(left->getType(), right->getType());
+        if (!isMultiplicationTypeCombinationValid(op, left->getType(), right->getType()))
+        {
+            return nullptr;
+        }
+    }
+
     TIntermBinary *node = new TIntermBinary(op, left, right);
     node->setLine(loc);
 
@@ -3688,6 +3740,14 @@
 {
     if (binaryOpCommonCheck(op, left, right, loc))
     {
+        if (op == EOpMulAssign)
+        {
+            op = TIntermBinary::GetMulAssignOpBasedOnOperands(left->getType(), right->getType());
+            if (!isMultiplicationTypeCombinationValid(op, left->getType(), right->getType()))
+            {
+                return nullptr;
+            }
+        }
         TIntermBinary *node = new TIntermBinary(op, left, right);
         node->setLine(loc);