Move some validation from IntermBinary::promote to ParseContext

src/compiler/translator/IntermNode.cpp

@@ -383,53 +383,13 @@ bool TIntermUnary::promote(TInfoSink &)
 // Establishes the type of the resultant operation, as well as
 // makes the operator the correct one for the operands.
 //
-// Returns false if operator can't work on operands.
+// For lots of operations it should already be established that the operand
+// combination is valid, but returns false if operator can't work on operands.
 //
 bool TIntermBinary::promote(TInfoSink &infoSink)
 {
     ASSERT(mLeft->isArray() == mRight->isArray());
 
-    // GLSL ES 2.0 does not support implicit type casting.
-    // So the basic type should usually match.
-    bool basicTypesMustMatch = true;
-
-    // Check ops which require integer / ivec parameters
-    switch (mOp)
-    {
-      case EOpBitShiftLeft:
-      case EOpBitShiftRight:
-      case EOpBitShiftLeftAssign:
-      case EOpBitShiftRightAssign:
-        // Unsigned can be bit-shifted by signed and vice versa, but we need to
-        // check that the basic type is an integer type.
-        basicTypesMustMatch = false;
-        if (!IsInteger(mLeft->getBasicType()) || !IsInteger(mRight->getBasicType()))
-        {
-            return false;
-        }
-        break;
-      case EOpBitwiseAnd:
-      case EOpBitwiseXor:
-      case EOpBitwiseOr:
-      case EOpBitwiseAndAssign:
-      case EOpBitwiseXorAssign:
-      case EOpBitwiseOrAssign:
-        // It is enough to check the type of only one operand, since later it
-        // is checked that the operand types match.
-        if (!IsInteger(mLeft->getBasicType()))
-        {
-            return false;
-        }
-        break;
-      default:
-        break;
-    }
-
-    if (basicTypesMustMatch && mLeft->getBasicType() != mRight->getBasicType())
-    {
-        return false;
-    }
-
     //
     // Base assumption:  just make the type the same as the left
     // operand.  Then only deviations from this need be coded.
@@ -474,12 +434,9 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
           // And and Or operate on conditionals
           //
           case EOpLogicalAnd:
+          case EOpLogicalXor:
           case EOpLogicalOr:
-            // Both operands must be of type bool.
-            if (mLeft->getBasicType() != EbtBool || mRight->getBasicType() != EbtBool)
-            {
-                return false;
-            }
+            ASSERT(mLeft->getBasicType() == EbtBool && mRight->getBasicType() == EbtBool);
             setType(TType(EbtBool, EbpUndefined));
             break;
 
@@ -616,6 +573,10 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
 
       case EOpAssign:
       case EOpInitialize:
+        // No more additional checks are needed.
+        ASSERT((mLeft->getNominalSize() == mRight->getNominalSize()) &&
+            (mLeft->getSecondarySize() == mRight->getSecondarySize()));
+        break;
       case EOpAdd:
       case EOpSub:
       case EOpDiv:
@@ -658,10 +619,6 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
                 mOp == EOpBitShiftLeft ||
                 mOp == EOpBitShiftRight))
                 return false;
-
-            // Operator cannot be of type pure assignment.
-            if (mOp == EOpAssign || mOp == EOpInitialize)
-                return false;
         }
 
         {
@@ -683,11 +640,8 @@ bool TIntermBinary::promote(TInfoSink &infoSink)
       case EOpGreaterThan:
       case EOpLessThanEqual:
       case EOpGreaterThanEqual:
-        if ((mLeft->getNominalSize() != mRight->getNominalSize()) ||
-            (mLeft->getSecondarySize() != mRight->getSecondarySize()))
-        {
-            return false;
-        }
+        ASSERT((mLeft->getNominalSize() == mRight->getNominalSize()) &&
+            (mLeft->getSecondarySize() == mRight->getSecondarySize()));
         setType(TType(EbtBool, EbpUndefined));
         break;
 

src/compiler/translator/ParseContext.cpp

@@ -2716,7 +2716,7 @@ TIntermTyped *TParseContext::addUnaryMathLValue(TOperator op, TIntermTyped *chil
     return addUnaryMath(op, child, loc);
 }
 
-bool TParseContext::binaryOpArrayCheck(TOperator op, TIntermTyped *left, TIntermTyped *right,
+bool TParseContext::binaryOpCommonCheck(TOperator op, TIntermTyped *left, TIntermTyped *right,
     const TSourceLoc &loc)
 {
     if (left->isArray() || right->isArray())
@@ -2750,13 +2750,74 @@ bool TParseContext::binaryOpArrayCheck(TOperator op, TIntermTyped *left, TInterm
             return false;
         }
     }
+
+    // Check ops which require integer / ivec parameters
+    bool isBitShift = false;
+    switch (op)
+    {
+      case EOpBitShiftLeft:
+      case EOpBitShiftRight:
+      case EOpBitShiftLeftAssign:
+      case EOpBitShiftRightAssign:
+        // Unsigned can be bit-shifted by signed and vice versa, but we need to
+        // check that the basic type is an integer type.
+        isBitShift = true;
+        if (!IsInteger(left->getBasicType()) || !IsInteger(right->getBasicType()))
+        {
+            return false;
+        }
+        break;
+      case EOpBitwiseAnd:
+      case EOpBitwiseXor:
+      case EOpBitwiseOr:
+      case EOpBitwiseAndAssign:
+      case EOpBitwiseXorAssign:
+      case EOpBitwiseOrAssign:
+        // It is enough to check the type of only one operand, since later it
+        // is checked that the operand types match.
+        if (!IsInteger(left->getBasicType()))
+        {
+            return false;
+        }
+        break;
+      default:
+        break;
+    }
+
+    // GLSL ES 1.00 and 3.00 do not support implicit type casting.
+    // So the basic type should usually match.
+    if (!isBitShift && left->getBasicType() != right->getBasicType())
+    {
+        return false;
+    }
+
+    // Check that type sizes match exactly on ops that require that:
+    switch(op)
+    {
+      case EOpAssign:
+      case EOpInitialize:
+      case EOpEqual:
+      case EOpNotEqual:
+      case EOpLessThan:
+      case EOpGreaterThan:
+      case EOpLessThanEqual:
+      case EOpGreaterThanEqual:
+        if ((left->getNominalSize() != right->getNominalSize()) ||
+            (left->getSecondarySize() != right->getSecondarySize()))
+        {
+            return false;
+        }
+      default:
+        break;
+    }
+
     return true;
 }
 
 TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op, TIntermTyped *left, TIntermTyped *right,
     const TSourceLoc &loc)
 {
-    if (!binaryOpArrayCheck(op, left, right, loc))
+    if (!binaryOpCommonCheck(op, left, right, loc))
         return nullptr;
 
     switch (op)
@@ -2809,12 +2870,6 @@ TIntermTyped *TParseContext::addBinaryMathInternal(TOperator op, TIntermTyped *l
         break;
     }
 
-    // This check is duplicated between here and node->promote() as an optimization.
-    if (left->getBasicType() != right->getBasicType() && op != EOpBitShiftLeft && op != EOpBitShiftRight)
-    {
-        return nullptr;
-    }
-
     return intermediate.addBinaryMath(op, left, right, loc);
 }
 
@@ -2849,7 +2904,7 @@ TIntermTyped *TParseContext::addBinaryMathBooleanResult(TOperator op, TIntermTyp
 TIntermTyped *TParseContext::createAssign(TOperator op, TIntermTyped *left, TIntermTyped *right,
     const TSourceLoc &loc)
 {
-    if (binaryOpArrayCheck(op, left, right, loc))
+    if (binaryOpCommonCheck(op, left, right, loc))
     {
         return intermediate.addAssign(op, left, right, loc);
     }

src/compiler/translator/ParseContext.h

@@ -192,8 +192,8 @@ struct TParseContext {
     TIntermTyped *createAssign(TOperator op, TIntermTyped *left, TIntermTyped *right,
         const TSourceLoc &loc);
 
-    // Return true if array-related checks pass
-    bool binaryOpArrayCheck(TOperator op, TIntermTyped *left, TIntermTyped *right,
+    // Return true if the checks pass
+    bool binaryOpCommonCheck(TOperator op, TIntermTyped *left, TIntermTyped *right,
         const TSourceLoc &loc);
 };