Move some validation from IntermBinary::promote to ParseContext
This makes the role of promote() in the system clearer and helps to make
the code more understandable, since more of the checks are in the same
logical place.
BUG=angleproject:952
TEST=angle_unittests, WebGL conformance tests
Change-Id: Idb2de927d872e46210d71cf6de06a6f8c1fc5da1
Reviewed-on: https://chromium-review.googlesource.com/260803
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 234f3c7..6b2eadb 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -383,53 +383,13 @@
// 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 @@
// 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 @@
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 @@
mOp == EOpBitShiftLeft ||
mOp == EOpBitShiftRight))
return false;
-
- // Operator cannot be of type pure assignment.
- if (mOp == EOpAssign || mOp == EOpInitialize)
- return false;
}
{
@@ -683,11 +640,8 @@
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;
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index a3fd403..d531bc4 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2716,7 +2716,7 @@
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 @@
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 @@
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::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);
}
diff --git a/src/compiler/translator/ParseContext.h b/src/compiler/translator/ParseContext.h
index 3d51d76..f32847e 100644
--- a/src/compiler/translator/ParseContext.h
+++ b/src/compiler/translator/ParseContext.h
@@ -192,8 +192,8 @@
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);
};