commit dca3e796d1ca46692090a6dfd27432722504af9c
author Olli Etuaho <oetuaho@nvidia.com> Thu Mar 26 13:24:04 2015 +0200
committer Olli Etuaho <oetuaho@nvidia.com> Thu Apr 02 15:58:59 2015 +0000
tree f6d0a08f038810a5c42327deb3b6a10c57a7e9fb
parent 7f67b030a8ae26e3ff54f6d33c655fc69c0b9fdb

Refactor unary math operator handling to clarify responsibilities

Shuffle the code around so that each part has a clear responsibility:
IntermUnary::promote is responsible for setting the return type of the
node, Intermediate::addUnaryMath is responsible for creating the node
object, and ParseContext::createUnaryMath is responsible for validating
the operand type.

This removes duplicated bool type check for logical not.

BUG=angleproject:952
TEST=angle_unittests, WebGL conformance tests

Change-Id: I9f5a0abb6434ad2730441ea9199ec3f5382ebcda
Reviewed-on: https://chromium-review.googlesource.com/262415
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>

src/compiler/translator/IntermNode.cpp

diff --git a/src/compiler/translator/IntermNode.cpp b/src/compiler/translator/IntermNode.cpp
index 6b2eadb..910a8b8 100644
--- a/src/compiler/translator/IntermNode.cpp
+++ b/src/compiler/translator/IntermNode.cpp
@@ -328,55 +328,34 @@
 // Make sure the type of a unary operator is appropriate for its
 // combination of operation and operand type.
 //
-// Returns false in nothing makes sense.
-//
-bool TIntermUnary::promote(TInfoSink &)
+void TIntermUnary::promote()
 {
     switch (mOp)
     {
-      case EOpLogicalNot:
-        if (mOperand->getBasicType() != EbtBool)
-            return false;
-        break;
-      // bit-wise not is already checked
-      case EOpBitwiseNot:
-        break;
-
-      case EOpNegative:
-      case EOpPositive:
-      case EOpPostIncrement:
-      case EOpPostDecrement:
-      case EOpPreIncrement:
-      case EOpPreDecrement:
-        if (mOperand->getBasicType() == EbtBool)
-            return false;
-        break;
-
-      // Operators for built-ins are already type checked against their prototype
-      // and some of them get the type of their return value assigned elsewhere.
+      // Some built-ins get the type of their return value assigned elsewhere.
       case EOpAny:
       case EOpAll:
       case EOpVectorLogicalNot:
+        break;
+      case EOpFloatBitsToInt:
+      case EOpFloatBitsToUint:
       case EOpIntBitsToFloat:
       case EOpUintBitsToFloat:
+      case EOpPackSnorm2x16:
+      case EOpPackUnorm2x16:
+      case EOpPackHalf2x16:
       case EOpUnpackSnorm2x16:
       case EOpUnpackUnorm2x16:
-      case EOpUnpackHalf2x16:
-        return true;
-
-      case EOpAbs:
-      case EOpSign:
+        mType.setPrecision(EbpHigh);
         break;
-
+      case EOpUnpackHalf2x16:
+        mType.setPrecision(EbpMedium);
+        break;
       default:
-        if (mOperand->getBasicType() != EbtFloat)
-            return false;
+        setType(mOperand->getType());
     }
 
-    setType(mOperand->getType());
     mType.setQualifier(EvqTemporary);
-
-    return true;
 }
 
 //

src/compiler/translator/IntermNode.h

diff --git a/src/compiler/translator/IntermNode.h b/src/compiler/translator/IntermNode.h
index f2de2d9..75879c1 100644
--- a/src/compiler/translator/IntermNode.h
+++ b/src/compiler/translator/IntermNode.h
@@ -383,7 +383,7 @@
 
     void setOperand(TIntermTyped *operand) { mOperand = operand; }
     TIntermTyped *getOperand() { return mOperand; }
-    bool promote(TInfoSink &);
+    void promote();
 
     void setUseEmulatedFunction() { mUseEmulatedFunction = true; }
     bool getUseEmulatedFunction() { return mUseEmulatedFunction; }

src/compiler/translator/Intermediate.cpp

diff --git a/src/compiler/translator/Intermediate.cpp b/src/compiler/translator/Intermediate.cpp
index 12fb5bd..b72a2bf 100644
--- a/src/compiler/translator/Intermediate.cpp
+++ b/src/compiler/translator/Intermediate.cpp
@@ -139,29 +139,7 @@
     TIntermUnary *node = new TIntermUnary(op);
     node->setLine(line);
     node->setOperand(child);
-
-    if (!node->promote(mInfoSink))
-        return 0;
-
-    switch (op)
-    {
-      case EOpFloatBitsToInt:
-      case EOpFloatBitsToUint:
-      case EOpIntBitsToFloat:
-      case EOpUintBitsToFloat:
-      case EOpPackSnorm2x16:
-      case EOpPackUnorm2x16:
-      case EOpPackHalf2x16:
-      case EOpUnpackSnorm2x16:
-      case EOpUnpackUnorm2x16:
-        node->getTypePointer()->setPrecision(EbpHigh);
-        break;
-      case EOpUnpackHalf2x16:
-        node->getTypePointer()->setPrecision(EbpMedium);
-        break;
-      default:
-        break;
-    }
+    node->promote();
 
     if (childTempConstant)
     {

src/compiler/translator/ParseContext.cpp

diff --git a/src/compiler/translator/ParseContext.cpp b/src/compiler/translator/ParseContext.cpp
index 77e70a7..00dbf5e 100644
--- a/src/compiler/translator/ParseContext.cpp
+++ b/src/compiler/translator/ParseContext.cpp
@@ -2730,10 +2730,12 @@
       case EOpNegative:
       case EOpPositive:
         if (child->getBasicType() == EbtStruct ||
+            child->getBasicType() == EbtBool ||
             child->isArray())
         {
             return nullptr;
         }
+      // Operators for built-ins are already type checked against their prototype.
       default:
         break;
     }