A bunch of semantic checks were missing for binary arithmetic operations. Refactor the "promote" logic to fix these.
git-svn-id: https://cvs.khronos.org/svn/repos/ogl/trunk/ecosystem/public/sdk/tools/glslang@21784 e7fa87d3-cd2b-0410-9028-fcbf551c1848
diff --git a/glslang/Include/Types.h b/glslang/Include/Types.h
index 621a3e8..e26ab4b 100644
--- a/glslang/Include/Types.h
+++ b/glslang/Include/Types.h
@@ -563,6 +563,7 @@
void setArrayInformationType(TType* t) { arrayInformationType = t; }
TType* getArrayInformationType() { return arrayInformationType; }
virtual bool isVector() const { return vectorSize > 1; }
+ virtual bool isScalar() const { return vectorSize == 1; }
const char* getBasicString() const {
return TType::getBasicString(basicType);
}
diff --git a/glslang/Include/intermediate.h b/glslang/Include/intermediate.h
index 299945f..f5a1d44 100644
--- a/glslang/Include/intermediate.h
+++ b/glslang/Include/intermediate.h
@@ -360,6 +360,7 @@
virtual bool isMatrix() const { return type.isMatrix(); }
virtual bool isArray() const { return type.isArray(); }
virtual bool isVector() const { return type.isVector(); }
+ virtual bool isScalar() const { return type.isScalar(); }
TString getCompleteString() const { return type.getCompleteString(); }
protected:
diff --git a/glslang/MachineIndependent/Intermediate.cpp b/glslang/MachineIndependent/Intermediate.cpp
index 9e3e04f..5ddd8e6 100644
--- a/glslang/MachineIndependent/Intermediate.cpp
+++ b/glslang/MachineIndependent/Intermediate.cpp
@@ -71,37 +71,11 @@
//
TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc line)
{
+ // No operations work on blocks
if (left->getType().getBasicType() == EbtBlock || right->getType().getBasicType() == EbtBlock)
return 0;
- switch (op) {
- case EOpLessThan:
- case EOpGreaterThan:
- case EOpLessThanEqual:
- case EOpGreaterThanEqual:
- if (left->getType().isMatrix() || left->getType().isArray() || left->getType().isVector() || left->getType().getBasicType() == EbtStruct) {
- return 0;
- }
- break;
- case EOpLogicalOr:
- case EOpLogicalXor:
- case EOpLogicalAnd:
- if (left->getType().getBasicType() != EbtBool || left->getType().isMatrix() || left->getType().isArray() || left->getType().isVector()) {
- return 0;
- }
- break;
- case EOpAdd:
- case EOpSub:
- case EOpDiv:
- case EOpMul:
- if (left->getType().getBasicType() == EbtStruct || left->getType().getBasicType() == EbtBool || left->getType().isArray())
- return 0;
- default: break; // some compilers want this
- }
-
- //
- // First try converting the children to compatible types.
- //
+ // Try converting the children's base types to compatible types.
TIntermTyped* child = addConversion(op, left->getType(), right);
if (child)
right = child;
@@ -153,6 +127,10 @@
//
TIntermTyped* TIntermediate::addAssign(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc line)
{
+ // No block assignment
+ if (left->getType().getBasicType() == EbtBlock || right->getType().getBasicType() == EbtBlock)
+ return 0;
+
//
// Like adding binary math, except the conversion can only go
// from right to left.
@@ -991,111 +969,166 @@
//
bool TIntermBinary::promote(TInfoSink& infoSink)
{
- //
- // Arrays have to be exact matches.
- //
- if ((left->isArray() || right->isArray()) && (left->getType() != right->getType()))
+ // Arrays and structures have to be exact matches.
+ if ((left->isArray() || right->isArray() || left->getBasicType() == EbtStruct || right->getBasicType() == EbtStruct)
+ && left->getType() != right->getType())
return false;
- //
// Base assumption: just make the type the same as the left
- // operand. Then only deviations from this need be coded.
- //
+ // operand. Only deviations from this will be coded.
setType(left->getType());
- type.getQualifier().storage = EvqTemporary;
+ type.getQualifier().clear();
- //
- // Array operations.
- //
- if (left->isArray()) {
+ // Finish all array and structure operations.
+ if (left->isArray() || left->getBasicType() == EbtStruct) {
switch (op) {
- //
- // Promote to conditional
- //
case EOpEqual:
case EOpNotEqual:
+ // Promote to conditional
setType(TType(EbtBool));
- break;
+
+ return true;
case EOpAssign:
- // array information was correctly set above
- break;
+ // Keep type from above
+
+ return true;
default:
return false;
}
+ }
- return true;
+ //
+ // We now have only scalars, vectors, and matrices to worry about.
+ //
+
+ // Do general type checks against individual operands (comparing left and right is coming up, checking mixed shapes after that)
+ switch (op) {
+ case EOpLessThan:
+ case EOpGreaterThan:
+ case EOpLessThanEqual:
+ case EOpGreaterThanEqual:
+ // Relational comparisons need matching numeric types and will promote to scalar Boolean.
+ if (left->getBasicType() == EbtBool || left->getType().isMatrix())
+ return false;
+
+ // Fall through
+
+ case EOpEqual:
+ case EOpNotEqual:
+ // All the above comparisons result in a bool (but not the vector compares)
+ setType(TType(EbtBool));
+ break;
+
+ case EOpLogicalAnd:
+ case EOpLogicalOr:
+ case EOpLogicalXor:
+ // logical ops operate only on scalar Booleans and will promote to scalar Boolean.
+ if (left->getBasicType() != EbtBool || left->isVector() || left->isMatrix())
+ return false;
+
+ setType(TType(EbtBool));
+ break;
+
+ case EOpRightShift:
+ case EOpLeftShift:
+ case EOpRightShiftAssign:
+ case EOpLeftShiftAssign:
+
+ case EOpMod:
+ case EOpModAssign:
+
+ case EOpAnd:
+ case EOpInclusiveOr:
+ case EOpExclusiveOr:
+ case EOpAndAssign:
+ case EOpInclusiveOrAssign:
+ case EOpExclusiveOrAssign:
+ // Check for integer-only operands.
+ if ( left->getBasicType() != EbtInt && left->getBasicType() != EbtUint ||
+ right->getBasicType() != EbtInt && right->getBasicType() != EbtUint)
+ return false;
+ if (left->isMatrix() || right->isMatrix())
+ return false;
+
+ break;
+
+ case EOpAdd:
+ case EOpSub:
+ case EOpDiv:
+ case EOpMul:
+ case EOpAddAssign:
+ case EOpSubAssign:
+ case EOpMulAssign:
+ case EOpDivAssign:
+ // check for non-Boolean operands
+ if (left->getBasicType() == EbtBool || right->getBasicType() == EbtBool)
+ return false;
+
+ default:
+ break;
}
- //
- // All scalars. Code after this test assumes this case is removed!
- //
- if (left->getVectorSize() == 1 && right->getVectorSize() == 1) {
+ // Compare left and right, and finish with the cases where the operand types must match
+ switch (op) {
+ case EOpLessThan:
+ case EOpGreaterThan:
+ case EOpLessThanEqual:
+ case EOpGreaterThanEqual:
- switch (op) {
+ case EOpEqual:
+ case EOpNotEqual:
- //
- // Promote to conditional
- //
- case EOpEqual:
- case EOpNotEqual:
- case EOpLessThan:
- case EOpGreaterThan:
- case EOpLessThanEqual:
- case EOpGreaterThanEqual:
- setType(TType(EbtBool));
- break;
+ case EOpLogicalAnd:
+ case EOpLogicalOr:
+ case EOpLogicalXor:
+ return left->getType() == right->getType();
- //
- // And and Or operate only on conditionals
- //
- case EOpLogicalAnd:
- case EOpLogicalOr:
- if (left->getBasicType() != EbtBool || right->getBasicType() != EbtBool)
- return false;
- setType(TType(EbtBool));
- break;
+ // no shifts: they can mix types (scalar int can shift a vector uint, etc.)
- //
- // Check for integer only operands.
- //
- case EOpMod:
- case EOpRightShift:
- case EOpLeftShift:
- case EOpAnd:
- case EOpInclusiveOr:
- case EOpExclusiveOr:
- if ( left->getBasicType() != EbtInt && left->getBasicType() != EbtUint ||
- right->getBasicType() != EbtInt && right->getBasicType() != EbtUint)
- return false;
- break;
- case EOpModAssign:
- case EOpAndAssign:
- case EOpInclusiveOrAssign:
- case EOpExclusiveOrAssign:
- case EOpLeftShiftAssign:
- case EOpRightShiftAssign:
- if ( left->getBasicType() != EbtInt && left->getBasicType() != EbtUint ||
- right->getBasicType() != EbtInt && right->getBasicType() != EbtUint)
- return false;
- // fall through
+ case EOpMod:
+ case EOpModAssign:
- //
- // Everything else should have matching types
- //
- default:
- if (left->getBasicType() != right->getBasicType() ||
- left->isMatrix() != right->isMatrix())
- return false;
- }
+ case EOpAnd:
+ case EOpInclusiveOr:
+ case EOpExclusiveOr:
+ case EOpAndAssign:
+ case EOpInclusiveOrAssign:
+ case EOpExclusiveOrAssign:
- return true;
+ case EOpAdd:
+ case EOpSub:
+ case EOpDiv:
+ case EOpAddAssign:
+ case EOpSubAssign:
+ case EOpDivAssign:
+ // Quick out in case the types do match
+ if (left->getType() == right->getType())
+ return true;
+
+ // Fall through
+
+ case EOpMul:
+ case EOpMulAssign:
+ // At least the basic type has to match
+ if (left->getBasicType() != right->getBasicType())
+ return false;
+
+ default:
+ break;
}
+ // Finish up handling the case where both operands are scalars.
+ if (! left->isVector() && ! left->isMatrix() &&
+ ! right->isVector() && ! right->isMatrix())
+ return true;
+
//
- // Can these two operands be combined?
+ // We now have a mix of scalars, vectors, or matrices, for non-relational operations.
//
+
+ // Can these two operands be combined, what is the resulting type?
TBasicType basicType = left->getBasicType();
switch (op) {
case EOpMul:
@@ -1130,7 +1163,7 @@
// leave as component product
} else if (left->isVector() || right->isVector()) {
op = EOpVectorTimesScalar;
- if (right->getVectorSize() > 1)
+ if (right->isVector())
setType(TType(basicType, EvqTemporary, right->getVectorSize()));
}
} else {
@@ -1170,10 +1203,20 @@
return false;
}
break;
+
+ case EOpRightShift:
+ case EOpLeftShift:
+ case EOpRightShiftAssign:
+ case EOpLeftShiftAssign:
+ if (right->isVector() && (! left->isVector() || right->getVectorSize() != left->getVectorSize()))
+ return false;
+ break;
+
case EOpAssign:
if (left->getVectorSize() != right->getVectorSize() || left->getMatrixCols() != right->getMatrixCols() || left->getMatrixRows() != right->getMatrixRows())
return false;
// fall through
+
case EOpAdd:
case EOpSub:
case EOpDiv:
@@ -1194,27 +1237,15 @@
setType(TType(basicType, EvqTemporary, right->getVectorSize(), right->getMatrixCols(), right->getMatrixRows()));
break;
- case EOpEqual:
- case EOpNotEqual:
- case EOpLessThan:
- case EOpGreaterThan:
- case EOpLessThanEqual:
- case EOpGreaterThanEqual:
- if (left->isMatrix() && right->isVector() ||
- left->isVector() && right->isMatrix() ||
- left->getBasicType() != right->getBasicType())
- return false;
- setType(TType(EbtBool));
- break;
-
default:
return false;
}
//
- // One more check for assignment. The Resulting type has to match the left operand.
+ // One more check for assignment.
//
switch (op) {
+ // The resulting type has to match the left operand.
case EOpAssign:
case EOpAddAssign:
case EOpSubAssign:
diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp
index 5861bad..52538fd 100644
--- a/glslang/MachineIndependent/ParseHelper.cpp
+++ b/glslang/MachineIndependent/ParseHelper.cpp
@@ -416,7 +416,7 @@
//
bool TParseContext::integerErrorCheck(TIntermTyped* node, const char* token)
{
- if ((node->getBasicType() == EbtInt || node->getBasicType() == EbtUint) && node->getVectorSize() == 1 && ! node->isArray())
+ if ((node->getBasicType() == EbtInt || node->getBasicType() == EbtUint) && node->isScalar() && ! node->isArray())
return false;
error(node->getLine(), "scalar integer expression required", token, "");
diff --git a/glslang/MachineIndependent/glslang.y b/glslang/MachineIndependent/glslang.y
index 0302e7b..02ae201 100644
--- a/glslang/MachineIndependent/glslang.y
+++ b/glslang/MachineIndependent/glslang.y
@@ -601,7 +601,6 @@
}
;
-// TODO: clean up: can we eliminate function_call_or_method and function_call_generic?
function_call_or_method
: function_call_generic {
$$ = $1;