Commit 32cfaf4b by alokp@chromium.org

TIntermBinary::promote() was incorrectly marking the type of result as const in…

TIntermBinary::promote() was incorrectly marking the type of result as const in some cases. The result can only be const if both operands are const. Also cleaned up the function to remove redundant/repeated checks. BUG=24 TEST=OpenGL ES 2.0 Conformance tests Review URL: http://codereview.appspot.com/1938047 git-svn-id: https://angleproject.googlecode.com/svn/trunk@385 736b8ea6-26fd-11df-bfd4-992fa37f6226
parent 94e1934d
...@@ -10,6 +10,7 @@ ...@@ -10,6 +10,7 @@
#include <float.h> #include <float.h>
#include <limits.h> #include <limits.h>
#include <algorithm>
#include "compiler/localintermediate.h" #include "compiler/localintermediate.h"
#include "compiler/QualifierAlive.h" #include "compiler/QualifierAlive.h"
...@@ -17,9 +18,10 @@ ...@@ -17,9 +18,10 @@
bool CompareStructure(const TType& leftNodeType, ConstantUnion* rightUnionArray, ConstantUnion* leftUnionArray); bool CompareStructure(const TType& leftNodeType, ConstantUnion* rightUnionArray, ConstantUnion* leftUnionArray);
TPrecision GetHighestPrecision( TPrecision left, TPrecision right ){ static TPrecision GetHigherPrecision( TPrecision left, TPrecision right ){
return left > right ? left : right; return left > right ? left : right;
} }
//////////////////////////////////////////////////////////////////////////// ////////////////////////////////////////////////////////////////////////////
// //
// First set of functions are to help build the intermediate representation. // First set of functions are to help build the intermediate representation.
...@@ -111,12 +113,6 @@ TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIn ...@@ -111,12 +113,6 @@ TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIn
TIntermConstantUnion *leftTempConstant = left->getAsConstantUnion(); TIntermConstantUnion *leftTempConstant = left->getAsConstantUnion();
TIntermConstantUnion *rightTempConstant = right->getAsConstantUnion(); TIntermConstantUnion *rightTempConstant = right->getAsConstantUnion();
if (leftTempConstant)
leftTempConstant = left->getAsConstantUnion();
if (rightTempConstant)
rightTempConstant = right->getAsConstantUnion();
// //
// See if we can fold constants. // See if we can fold constants.
// //
...@@ -764,16 +760,9 @@ bool TIntermUnary::promote(TInfoSink&) ...@@ -764,16 +760,9 @@ bool TIntermUnary::promote(TInfoSink&)
// //
bool TIntermBinary::promote(TInfoSink& infoSink) bool TIntermBinary::promote(TInfoSink& infoSink)
{ {
int size = left->getNominalSize(); // GLSL ES 2.0 does not support implicit type casting.
if (right->getNominalSize() > size) // So the basic type should always match.
size = right->getNominalSize(); if (left->getBasicType() != right->getBasicType())
TBasicType type = left->getBasicType();
//
// Arrays have to be exact matches.
//
if ((left->isArray() || right->isArray()) && (left->getType() != right->getType()))
return false; return false;
// //
...@@ -782,16 +771,27 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -782,16 +771,27 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
// //
setType(left->getType()); setType(left->getType());
TPrecision highestPrecision = GetHighestPrecision(left->getPrecision(), right->getPrecision()); // The result gets promoted to the highest precision.
getTypePointer()->changePrecision(highestPrecision); TPrecision higherPrecision = GetHigherPrecision(left->getPrecision(), right->getPrecision());
getTypePointer()->changePrecision(higherPrecision);
// Binary operations results in temporary variables unless both
// operands are const.
if (left->getQualifier() != EvqConst || right->getQualifier() != EvqConst) {
getTypePointer()->changeQualifier(EvqTemporary);
}
// //
// Array operations. // Array operations.
// //
if (left->isArray()) { if (left->isArray() || right->isArray()) {
//
// Arrays types have to be exact matches.
//
if (left->getType() != right->getType())
return false;
switch (op) { switch (op) {
// //
// Promote to conditional // Promote to conditional
// //
...@@ -812,17 +812,16 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -812,17 +812,16 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
default: default:
return false; return false;
} }
return true; return true;
} }
int size = std::max(left->getNominalSize(), right->getNominalSize());
// //
// All scalars. Code after this test assumes this case is removed! // All scalars. Code after this test assumes this case is removed!
// //
if (size == 1) { if (size == 1) {
switch (op) { switch (op) {
// //
// Promote to conditional // Promote to conditional
// //
...@@ -840,33 +839,36 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -840,33 +839,36 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
// //
case EOpLogicalAnd: case EOpLogicalAnd:
case EOpLogicalOr: case EOpLogicalOr:
// Both operands must be of type bool.
if (left->getBasicType() != EbtBool || right->getBasicType() != EbtBool) if (left->getBasicType() != EbtBool || right->getBasicType() != EbtBool)
return false; return false;
setType(TType(EbtBool, EbpUndefined)); setType(TType(EbtBool, EbpUndefined));
break; break;
//
// Everything else should have matching types
//
default: default:
if (left->getBasicType() != right->getBasicType() || break;
left->isMatrix() != right->isMatrix())
return false;
} }
return true; return true;
} }
// // If we reach here, at least one of the operands is vector or matrix.
// The other operand could be a scalar, vector, or matrix.
// Are the sizes compatible? // Are the sizes compatible?
// //
if ( left->getNominalSize() != size && left->getNominalSize() != 1 || if (left->getNominalSize() != right->getNominalSize()) {
right->getNominalSize() != size && right->getNominalSize() != 1) // If the nominal size of operands do not match:
return false; // One of them must be scalar.
if (left->getNominalSize() != 1 && right->getNominalSize() != 1)
return false;
// Operator cannot be of type pure assignment.
if (op == EOpAssign || op == EOpInitialize)
return false;
}
// //
// Can these two operands be combined? // Can these two operands be combined?
// //
TBasicType basicType = left->getBasicType();
switch (op) { switch (op) {
case EOpMul: case EOpMul:
if (!left->isMatrix() && right->isMatrix()) { if (!left->isMatrix() && right->isMatrix()) {
...@@ -874,12 +876,12 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -874,12 +876,12 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
op = EOpVectorTimesMatrix; op = EOpVectorTimesMatrix;
else { else {
op = EOpMatrixTimesScalar; op = EOpMatrixTimesScalar;
setType(TType(type, highestPrecision, EvqTemporary, size, true)); setType(TType(basicType, higherPrecision, EvqTemporary, size, true));
} }
} else if (left->isMatrix() && !right->isMatrix()) { } else if (left->isMatrix() && !right->isMatrix()) {
if (right->isVector()) { if (right->isVector()) {
op = EOpMatrixTimesVector; op = EOpMatrixTimesVector;
setType(TType(type, highestPrecision, EvqTemporary, size, false)); setType(TType(basicType, higherPrecision, EvqTemporary, size, false));
} else { } else {
op = EOpMatrixTimesScalar; op = EOpMatrixTimesScalar;
} }
...@@ -890,7 +892,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -890,7 +892,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
// leave as component product // leave as component product
} else if (left->isVector() || right->isVector()) { } else if (left->isVector() || right->isVector()) {
op = EOpVectorTimesScalar; op = EOpVectorTimesScalar;
setType(TType(type, highestPrecision, EvqTemporary, size, false)); setType(TType(basicType, higherPrecision, EvqTemporary, size, false));
} }
} else { } else {
infoSink.info.message(EPrefixInternalError, "Missing elses", getLine()); infoSink.info.message(EPrefixInternalError, "Missing elses", getLine());
...@@ -919,18 +921,16 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -919,18 +921,16 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
if (! left->isVector()) if (! left->isVector())
return false; return false;
op = EOpVectorTimesScalarAssign; op = EOpVectorTimesScalarAssign;
setType(TType(type, highestPrecision, EvqTemporary, size, false)); setType(TType(basicType, higherPrecision, EvqTemporary, size, false));
} }
} else { } else {
infoSink.info.message(EPrefixInternalError, "Missing elses", getLine()); infoSink.info.message(EPrefixInternalError, "Missing elses", getLine());
return false; return false;
} }
break; break;
case EOpAssign: case EOpAssign:
case EOpInitialize: case EOpInitialize:
if (left->getNominalSize() != right->getNominalSize())
return false;
// fall through
case EOpAdd: case EOpAdd:
case EOpSub: case EOpSub:
case EOpDiv: case EOpDiv:
...@@ -938,10 +938,9 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -938,10 +938,9 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
case EOpSubAssign: case EOpSubAssign:
case EOpDivAssign: case EOpDivAssign:
if (left->isMatrix() && right->isVector() || if (left->isMatrix() && right->isVector() ||
left->isVector() && right->isMatrix() || left->isVector() && right->isMatrix())
left->getBasicType() != right->getBasicType())
return false; return false;
setType(TType(type, highestPrecision, EvqTemporary, size, left->isMatrix() || right->isMatrix())); setType(TType(basicType, higherPrecision, EvqTemporary, size, left->isMatrix() || right->isMatrix()));
break; break;
case EOpEqual: case EOpEqual:
...@@ -951,8 +950,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -951,8 +950,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
case EOpLessThanEqual: case EOpLessThanEqual:
case EOpGreaterThanEqual: case EOpGreaterThanEqual:
if (left->isMatrix() && right->isVector() || if (left->isMatrix() && right->isVector() ||
left->isVector() && right->isMatrix() || left->isVector() && right->isMatrix())
left->getBasicType() != right->getBasicType())
return false; return false;
setType(TType(EbtBool, EbpUndefined)); setType(TType(EbtBool, EbpUndefined));
break; break;
...@@ -960,24 +958,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink) ...@@ -960,24 +958,7 @@ bool TIntermBinary::promote(TInfoSink& infoSink)
default: default:
return false; return false;
} }
//
// One more check for assignment. The Resulting type has to match the left operand.
//
switch (op) {
case EOpAssign:
case EOpInitialize:
case EOpAddAssign:
case EOpSubAssign:
case EOpMulAssign:
case EOpDivAssign:
if (getType() != left->getType())
return false;
break;
default:
break;
}
return true; return true;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment