Refactor binary node creation
1. Simplify code by using asserts instead of adding internal errors to
log.
2. Add a TIntermBinary constructor that takes left and right operand
nodes as parameters.
3. Remove TIntermediate functions with trivial functionality.
BUG=angleproject:952
TEST=angle_unittests
Change-Id: I2e0e52160c9377d8efcf15f14fd59f01cb41bd83
Reviewed-on: https://chromium-review.googlesource.com/372720
Commit-Queue: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 2ce85e0..83f8e00 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -17,6 +17,7 @@
#include "common/mathutil.h"
#include "common/matrix_utils.h"
+#include "compiler/translator/Diagnostics.h"
#include "compiler/translator/HashNames.h"
#include "compiler/translator/IntermNode.h"
#include "compiler/translator/SymbolTable.h"
@@ -565,7 +566,7 @@
// 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)
+bool TIntermBinary::promote()
{
ASSERT(mLeft->isArray() == mRight->isArray());
@@ -686,8 +687,7 @@
}
else
{
- infoSink.info.message(EPrefixInternalError, getLine(),
- "Missing elses");
+ UNREACHABLE();
return false;
}
@@ -744,8 +744,7 @@
}
else
{
- infoSink.info.message(EPrefixInternalError, getLine(),
- "Missing elses");
+ UNREACHABLE();
return false;
}
@@ -836,7 +835,7 @@
return true;
}
-TIntermTyped *TIntermBinary::fold(TInfoSink &infoSink)
+TIntermTyped *TIntermBinary::fold(TDiagnostics *diagnostics)
{
TIntermConstantUnion *leftConstant = mLeft->getAsConstantUnion();
TIntermConstantUnion *rightConstant = mRight->getAsConstantUnion();
@@ -844,7 +843,7 @@
{
return nullptr;
}
- TConstantUnion *constArray = leftConstant->foldBinary(mOp, rightConstant, infoSink);
+ TConstantUnion *constArray = leftConstant->foldBinary(mOp, rightConstant, diagnostics);
// Nodes may be constant folded without being qualified as constant.
TQualifier resultQualifier = EvqConst;
@@ -917,7 +916,9 @@
//
// Returns the constant value to keep using or nullptr.
//
-TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink)
+TConstantUnion *TIntermConstantUnion::foldBinary(TOperator op,
+ TIntermConstantUnion *rightNode,
+ TDiagnostics *diagnostics)
{
const TConstantUnion *leftArray = getUnionArrayPointer();
const TConstantUnion *rightArray = rightNode->getUnionArrayPointer();
@@ -966,14 +967,7 @@
case EOpMatrixTimesMatrix:
{
- if (getType().getBasicType() != EbtFloat ||
- rightNode->getBasicType() != EbtFloat)
- {
- infoSink.info.message(
- EPrefixInternalError, getLine(),
- "Constant Folding cannot be done for matrix multiply");
- return nullptr;
- }
+ ASSERT(getType().getBasicType() == EbtFloat && rightNode->getBasicType() == EbtFloat);
const int leftCols = getCols();
const int leftRows = getRows();
@@ -1011,8 +1005,8 @@
case EbtFloat:
if (rightArray[i] == 0.0f)
{
- infoSink.info.message(EPrefixWarning, getLine(),
- "Divide by zero error during constant folding");
+ diagnostics->warning(
+ getLine(), "Divide by zero error during constant folding", "/", "");
resultArray[i].setFConst(leftArray[i].getFConst() < 0 ? -FLT_MAX : FLT_MAX);
}
else
@@ -1025,8 +1019,8 @@
case EbtInt:
if (rightArray[i] == 0)
{
- infoSink.info.message(EPrefixWarning, getLine(),
- "Divide by zero error during constant folding");
+ diagnostics->warning(
+ getLine(), "Divide by zero error during constant folding", "/", "");
resultArray[i].setIConst(INT_MAX);
}
else
@@ -1046,8 +1040,8 @@
case EbtUInt:
if (rightArray[i] == 0)
{
- infoSink.info.message(EPrefixWarning, getLine(),
- "Divide by zero error during constant folding");
+ diagnostics->warning(
+ getLine(), "Divide by zero error during constant folding", "/", "");
resultArray[i].setUConst(UINT_MAX);
}
else
@@ -1065,9 +1059,8 @@
break;
default:
- infoSink.info.message(EPrefixInternalError, getLine(),
- "Constant folding cannot be done for \"/\"");
- return nullptr;
+ UNREACHABLE();
+ return nullptr;
}
}
}
@@ -1075,12 +1068,7 @@
case EOpMatrixTimesVector:
{
- if (rightNode->getBasicType() != EbtFloat)
- {
- infoSink.info.message(EPrefixInternalError, getLine(),
- "Constant Folding cannot be done for matrix times vector");
- return nullptr;
- }
+ ASSERT(rightNode->getBasicType() == EbtFloat);
const int matrixCols = getCols();
const int matrixRows = getRows();
@@ -1102,12 +1090,7 @@
case EOpVectorTimesMatrix:
{
- if (getType().getBasicType() != EbtFloat)
- {
- infoSink.info.message(EPrefixInternalError, getLine(),
- "Constant Folding cannot be done for vector times matrix");
- return nullptr;
- }
+ ASSERT(getType().getBasicType() == EbtFloat);
const int matrixCols = rightNode->getType().getCols();
const int matrixRows = rightNode->getType().getRows();
@@ -1149,18 +1132,11 @@
case EOpLogicalXor:
{
+ ASSERT(getType().getBasicType() == EbtBool);
resultArray = new TConstantUnion[objectSize];
for (size_t i = 0; i < objectSize; i++)
{
- switch (getType().getBasicType())
- {
- case EbtBool:
- resultArray[i].setBConst(leftArray[i] != rightArray[i]);
- break;
- default:
- UNREACHABLE();
- break;
- }
+ resultArray[i].setBConst(leftArray[i] != rightArray[i]);
}
}
break;
@@ -1240,10 +1216,8 @@
break;
default:
- infoSink.info.message(
- EPrefixInternalError, getLine(),
- "Invalid operator for constant folding");
- return nullptr;
+ UNREACHABLE();
+ return nullptr;
}
return resultArray;
}
diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index e06652f..7aae484 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -27,6 +27,8 @@
#include "compiler/translator/Operator.h"
#include "compiler/translator/Types.h"
+class TDiagnostics;
+
class TIntermTraverser;
class TIntermAggregate;
class TIntermBinary;
@@ -349,7 +351,9 @@
void traverse(TIntermTraverser *it) override;
bool replaceChildNode(TIntermNode *, TIntermNode *) override { return false; }
- TConstantUnion *foldBinary(TOperator op, TIntermConstantUnion *rightNode, TInfoSink &infoSink);
+ TConstantUnion *foldBinary(TOperator op,
+ TIntermConstantUnion *rightNode,
+ TDiagnostics *diagnostics);
TConstantUnion *foldUnaryWithDifferentReturnType(TOperator op, TInfoSink &infoSink);
TConstantUnion *foldUnaryWithSameReturnType(TOperator op, TInfoSink &infoSink);
@@ -406,6 +410,11 @@
: TIntermOperator(op),
mAddIndexClamp(false) {}
+ TIntermBinary(TOperator op, TIntermTyped *left, TIntermTyped *right)
+ : TIntermOperator(op), mLeft(left), mRight(right), mAddIndexClamp(false)
+ {
+ }
+
TIntermTyped *deepCopy() const override { return new TIntermBinary(*this); }
TIntermBinary *getAsBinaryNode() override { return this; };
@@ -421,8 +430,8 @@
void setRight(TIntermTyped *node) { mRight = node; }
TIntermTyped *getLeft() const { return mLeft; }
TIntermTyped *getRight() const { return mRight; }
- bool promote(TInfoSink &);
- TIntermTyped *fold(TInfoSink &infoSink);
+ bool promote();
+ TIntermTyped *fold(TDiagnostics *diagnostics);
void setAddIndexClamp() { mAddIndexClamp = true; }
bool getAddIndexClamp() { return mAddIndexClamp; }
diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index 9a2e1c5..b6fefa4 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -38,53 +38,6 @@
}
//
-// Connect two nodes with a new parent that does a binary operation on the nodes.
-//
-// Returns the added node.
-//
-TIntermTyped *TIntermediate::addBinaryMath(
- TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
-{
- //
- // Need a new node holding things together then. Make
- // one and promote it to the right type.
- //
- TIntermBinary *node = new TIntermBinary(op);
- node->setLine(line);
-
- node->setLeft(left);
- node->setRight(right);
- if (!node->promote(mInfoSink))
- return NULL;
-
- // See if we can fold constants.
- TIntermTyped *foldedNode = node->fold(mInfoSink);
- if (foldedNode)
- return foldedNode;
-
- return node;
-}
-
-//
-// Connect two nodes through an assignment.
-//
-// Returns the added node.
-//
-TIntermTyped *TIntermediate::addAssign(
- TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &line)
-{
- TIntermBinary *node = new TIntermBinary(op);
- node->setLine(line);
-
- node->setLeft(left);
- node->setRight(right);
- if (!node->promote(mInfoSink))
- return NULL;
-
- return node;
-}
-
-//
// Connect two nodes through an index operator, where the left node is the base
// of an array or struct, and the right node is a direct or indirect offset.
//
diff --git a/src/compiler/translator/Intermediate.h b/src/compiler/translator/Intermediate.h
index f723fc7..339daa3 100644
--- a/src/compiler/translator/Intermediate.h
+++ b/src/compiler/translator/Intermediate.h
@@ -28,10 +28,6 @@
TIntermSymbol *addSymbol(
int id, const TString &, const TType &, const TSourceLoc &);
- TIntermTyped *addBinaryMath(
- TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
- TIntermTyped *addAssign(
- TOperator op, TIntermTyped *left, TIntermTyped *right, const TSourceLoc &);
TIntermTyped *addIndex(
TOperator op, TIntermTyped *base, TIntermTyped *index, const TSourceLoc &);
TIntermTyped *addUnaryMath(
diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 5a79d2a..1561382 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -3631,7 +3631,18 @@
break;
}
- return intermediate.addBinaryMath(op, left, right, loc);
+ TIntermBinary *node = new TIntermBinary(op, left, right);
+ node->setLine(loc);
+
+ if (!node->promote())
+ return nullptr;
+
+ // See if we can fold constants.
+ TIntermTyped *foldedNode = node->fold(&mDiagnostics);
+ if (foldedNode)
+ return foldedNode;
+
+ return node;
}
TIntermTyped *TParseContext::addBinaryMath(TOperator op,
@@ -3674,7 +3685,13 @@
{
if (binaryOpCommonCheck(op, left, right, loc))
{
- return intermediate.addAssign(op, left, right, loc);
+ TIntermBinary *node = new TIntermBinary(op, left, right);
+ node->setLine(loc);
+
+ if (!node->promote())
+ return nullptr;
+
+ return node;
}
return nullptr;
}
diff --git a/src/compiler/translator/RemovePow.cpp b/src/compiler/translator/RemovePow.cpp
index d55e124..48fc1f9 100644
--- a/src/compiler/translator/RemovePow.cpp
+++ b/src/compiler/translator/RemovePow.cpp
@@ -52,8 +52,6 @@
{
if (IsProblematicPow(node))
{
- TInfoSink nullSink;
-
TIntermTyped *x = node->getSequence()->at(0)->getAsTyped();
TIntermTyped *y = node->getSequence()->at(1)->getAsTyped();
@@ -62,11 +60,9 @@
log->setLine(node->getLine());
log->setType(x->getType());
- TIntermBinary *mul = new TIntermBinary(EOpMul);
- mul->setLeft(y);
- mul->setRight(log);
+ TIntermBinary *mul = new TIntermBinary(EOpMul, y, log);
mul->setLine(node->getLine());
- bool valid = mul->promote(nullSink);
+ bool valid = mul->promote();
UNUSED_ASSERTION_VARIABLE(valid);
ASSERT(valid);
