HLSL: Fix unary and binary operator type conversion issues

Test/hlsl.logical.binary.frag

+struct PS_OUTPUT
+{
+    float4 Color : SV_Target0;
+};
+
+uniform int    ival;
+uniform int4   ival4;
+uniform float  fval;
+uniform float4 fval4;
+
+PS_OUTPUT main()
+{
+    if (ival && fval);
+    if (ival || fval);
+
+    PS_OUTPUT psout;
+    psout.Color = 1.0;
+    return psout;
+}
+
+

Test/hlsl.logical.unary.frag

+struct PS_OUTPUT
+{
+    float4 Color : SV_Target0;
+};
+
+uniform int    ival;
+uniform int4   ival4;
+uniform float  fval;
+uniform float4 fval4;
+
+PS_OUTPUT main()
+{
+    !ival;  // scalar int
+    !ival4; // vector int
+
+    !fval;  // scalar float
+    !fval4; // vector float
+    
+    if (ival);
+    if (fval);
+    if (!ival);
+    if (!fval);
+
+    PS_OUTPUT psout;
+    psout.Color = 1.0;
+    return psout;
+}
+
+

Test/hlsl.promote.binary.frag

+struct PS_OUTPUT
+{
+    float4 Color : SV_Target0;
+};
+
+uniform bool   bval;
+uniform bool4  bval4;
+uniform int    ival;
+uniform int4   ival4;
+uniform float  fval;
+uniform float4 fval4;
+
+PS_OUTPUT main()
+{
+    ival  % fval;
+    ival4 % fval4;
+
+    bval  % fval;
+    bval4 % fval4;
+
+    int l_int = 1;
+    l_int %= fval;
+
+    PS_OUTPUT psout;
+    psout.Color = 0;
+    return psout;
+}
+

glslang/Include/intermediate.h

@@ -52,6 +52,8 @@
 
 namespace glslang {
 
+class TIntermediate;
+
 //
 // Operators used by the high-level (parse tree) representation.
 //
@@ -845,7 +847,7 @@ public:
     virtual       TIntermOperator* getAsOperator()       { return this; }
     virtual const TIntermOperator* getAsOperator() const { return this; }
     TOperator getOp() const { return op; }
-    virtual bool promote() { return true; }
+    virtual bool promote(TIntermediate&) { return true; }
     bool modifiesState() const;
     bool isConstructor() const;
     bool isTexture()  const { return op > EOpTextureGuardBegin  && op < EOpTextureGuardEnd; }
@@ -1024,7 +1026,7 @@ public:
     virtual TIntermTyped* getRight() const { return right; }
     virtual       TIntermBinary* getAsBinaryNode()       { return this; }
     virtual const TIntermBinary* getAsBinaryNode() const { return this; }
-    virtual bool promote();
+    virtual bool promote(TIntermediate&);
     virtual void updatePrecision();
 protected:
     TIntermTyped* left;
@@ -1044,7 +1046,7 @@ public:
     virtual const TIntermTyped* getOperand() const { return operand; }
     virtual       TIntermUnary* getAsUnaryNode()       { return this; }
     virtual const TIntermUnary* getAsUnaryNode() const { return this; }
-    virtual bool promote();
+    virtual bool promote(TIntermediate&);
     virtual void updatePrecision();
 protected:
     TIntermTyped* operand;

glslang/MachineIndependent/Intermediate.cpp

@@ -137,7 +137,7 @@ TIntermTyped* TIntermediate::addBinaryMath(TOperator op, TIntermTyped* left, TIn
     // one and promote it to the right type.
     //
     TIntermBinary* node = addBinaryNode(op, left, right, loc);
-    if (! node->promote())
+    if (! node->promote(*this))
         return 0;
 
     node->updatePrecision();
@@ -246,7 +246,7 @@ TIntermTyped* TIntermediate::addAssign(TOperator op, TIntermTyped* left, TInterm
     // build the node
     TIntermBinary* node = addBinaryNode(op, left, right, loc);
 
-    if (! node->promote())
+    if (! node->promote(*this))
         return nullptr;
 
     node->updatePrecision();
@@ -282,6 +282,10 @@ TIntermTyped* TIntermediate::addUnaryMath(TOperator op, TIntermTyped* child, TSo
 
     switch (op) {
     case EOpLogicalNot:
+        if (source == EShSourceHlsl) {
+            break; // HLSL can promote logical not
+        }
+ 
         if (child->getType().getBasicType() != EbtBool || child->getType().isMatrix() || child->getType().isArray() || child->getType().isVector()) {
             return 0;
         }
@@ -349,7 +353,7 @@ TIntermTyped* TIntermediate::addUnaryMath(TOperator op, TIntermTyped* child, TSo
     //
     TIntermUnary* node = addUnaryNode(op, child, loc);
 
-    if (! node->promote())
+    if (! node->promote(*this))
         return 0;
 
     node->updatePrecision();
@@ -555,6 +559,10 @@ TIntermTyped* TIntermediate::addConversion(TOperator op, const TType& type, TInt
     case EOpAndAssign:
     case EOpInclusiveOrAssign:
     case EOpExclusiveOrAssign:
+    case EOpLogicalNot:
+    case EOpLogicalAnd:
+    case EOpLogicalOr:
+    case EOpLogicalXor:
 
     case EOpFunctionCall:
     case EOpReturn:
@@ -841,6 +849,10 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
             case EOpModAssign:               // ... 
             case EOpReturn:                  // function returns can also perform arbitrary conversions
             case EOpFunctionCall:            // conversion of a calling parameter
+            case EOpLogicalNot:
+            case EOpLogicalAnd:
+            case EOpLogicalOr:
+            case EOpLogicalXor:
                 return true;
             default:
                 break;
@@ -873,6 +885,8 @@ bool TIntermediate::canImplicitlyPromote(TBasicType from, TBasicType to, TOperat
         case EbtFloat16:
 #endif
             return true;
+        case EbtBool:
+            return (source == EShSourceHlsl);
         default:
             return false;
         }
@@ -1716,13 +1730,20 @@ bool TIntermOperator::isConstructor() const
 //
 // Returns false in nothing makes sense.
 //
-bool TIntermUnary::promote()
+bool TIntermUnary::promote(TIntermediate& intermediate)
 {
     switch (op) {
     case EOpLogicalNot:
-        if (operand->getBasicType() != EbtBool)
+        // Convert operand to a boolean type
+        if (operand->getBasicType() != EbtBool) {
+            // Add constructor to boolean type. If that fails, we can't do it, so return false.
+            TIntermTyped* converted = intermediate.convertToBasicType(op, EbtBool, operand); 
+            if (converted == nullptr)
+                return false;
 
-            return false;
+            // Use the result of converting the node to a bool.
+            operand = converted;
+        }
         break;
     case EOpBitwiseNot:
         if (operand->getBasicType() != EbtInt &&
@@ -1774,13 +1795,31 @@ void TIntermUnary::updatePrecision()
     }
 }
 
+// If it is not already, convert this node to the given basic type.
+TIntermTyped* TIntermediate::convertToBasicType(TOperator op, TBasicType basicType, TIntermTyped* node) const
+{
+    if (node == nullptr)
+        return nullptr;
+
+    // It's already this basic type: nothing needs to be done, so use the node directly.
+    if (node->getBasicType() == basicType)
+        return node;
+
+    const TType& type = node->getType();
+    const TType newType(basicType, type.getQualifier().storage,
+                        type.getVectorSize(), type.getMatrixCols(), type.getMatrixRows(), type.isVector());
+
+    // Add constructor to the right vectorness of the right type. If that fails, we can't do it, so return nullptr.
+    return addConversion(op, newType, node);
+}
+
 //
 // 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.
 //
-bool TIntermBinary::promote()
+bool TIntermBinary::promote(TIntermediate& intermediate)
 {
     // Arrays and structures have to be exact matches.
     if ((left->isArray() || right->isArray() || left->getBasicType() == EbtStruct || right->getBasicType() == EbtStruct)
@@ -1843,6 +1882,15 @@ bool TIntermBinary::promote()
     case EOpLogicalAnd:
     case EOpLogicalOr:
     case EOpLogicalXor:
+        if (intermediate.getSource() == EShSourceHlsl) {
+            TIntermTyped* convertedL = intermediate.convertToBasicType(op, EbtBool, left);
+            TIntermTyped* convertedR = intermediate.convertToBasicType(op, EbtBool, right);
+            if (convertedL == nullptr || convertedR == nullptr)
+                return false;
+            left = convertedL;
+            right = convertedR;
+        }
+
         // logical ops operate only on scalar Booleans and will promote to scalar Boolean.
         if (left->getBasicType() != EbtBool || left->isVector() || left->isMatrix())
             return false;
@@ -1864,6 +1912,9 @@ bool TIntermBinary::promote()
     case EOpAndAssign:
     case EOpInclusiveOrAssign:
     case EOpExclusiveOrAssign:
+        if (intermediate.getSource() == EShSourceHlsl)
+            break;
+
         // Check for integer-only operands.
         if ((left->getBasicType() != EbtInt &&  left->getBasicType() != EbtUint &&
              left->getBasicType() != EbtInt64 &&  left->getBasicType() != EbtUint64) ||
@@ -2051,6 +2102,7 @@ bool TIntermBinary::promote()
     case EOpAndAssign:
     case EOpInclusiveOrAssign:
     case EOpExclusiveOrAssign:
+
         if ((left->isMatrix() && right->isVector()) ||
             (left->isVector() && right->isMatrix()) ||
             left->getBasicType() != right->getBasicType())

glslang/MachineIndependent/localintermediate.h

@@ -246,6 +246,9 @@ public:
     TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, TSourceLoc) const;
     TIntermUnary* addUnaryNode(TOperator op, TIntermTyped* child, TSourceLoc, const TType&) const;
 
+    // Add conversion from node's type to given basic type.
+    TIntermTyped* convertToBasicType(TOperator op, TBasicType basicType, TIntermTyped* node) const;
+
     // Constant folding (in Constant.cpp)
     TIntermTyped* fold(TIntermAggregate* aggrNode);
     TIntermTyped* foldConstructor(TIntermAggregate* aggrNode);
@@ -390,7 +393,7 @@ protected:
     bool userOutputUsed() const;
     static int getBaseAlignmentScalar(const TType&, int& size);
     bool isSpecializationOperation(const TIntermOperator&) const;
-
+    
     const EShLanguage language;  // stage, known at construction time
     EShSource source;            // source language, known a bit later
     std::string entryPointName;

gtests/Hlsl.FromFile.cpp

@@ -142,6 +142,8 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.load.rwtexture.array.dx10.frag", "main"},
         {"hlsl.load.offset.dx10.frag", "main"},
         {"hlsl.load.offsetarray.dx10.frag", "main"},
+        {"hlsl.logical.unary.frag", "main"},
+        {"hlsl.logical.binary.frag", "main"},
         {"hlsl.multiEntry.vert", "RealEntrypoint"},
         {"hlsl.multiReturn.frag", "main"},
         {"hlsl.matrixindex.frag", "main"},
@@ -149,6 +151,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.overload.frag", "PixelShaderFunction"},
         {"hlsl.pp.line.frag", "main"},
         {"hlsl.precise.frag", "main"},
+        {"hlsl.promote.binary.frag", "main"},
         {"hlsl.promotions.frag", "main"},
         {"hlsl.rw.bracket.frag", "main"},
         {"hlsl.rw.scalar.bracket.frag", "main"},
@@ -179,7 +182,7 @@ INSTANTIATE_TEST_CASE_P(
         {"hlsl.samplelevel.basic.dx10.vert", "main"},
         {"hlsl.samplelevel.offset.dx10.frag", "main"},
         {"hlsl.samplelevel.offsetarray.dx10.frag", "main"},
-        { "hlsl.sample.sub-vec4.dx10.frag", "main"},
+        {"hlsl.sample.sub-vec4.dx10.frag", "main"},
         {"hlsl.semicolons.frag", "main"},
         {"hlsl.shapeConv.frag", "main"},
         {"hlsl.shapeConvRet.frag", "main"},

hlsl/hlslGrammar.cpp

@@ -1935,7 +1935,10 @@ bool HlslGrammar::acceptUnaryExpression(TIntermTyped*& node)
         return true;
 
     node = intermediate.addUnaryMath(unaryOp, node, loc);
-    node = parseContext.handleLvalue(loc, "unary operator", node);
+
+    // These unary ops require lvalues
+    if (unaryOp == EOpPreIncrement || unaryOp == EOpPreDecrement)
+        node = parseContext.handleLvalue(loc, "unary operator", node);
 
     return node != nullptr;
 }

hlsl/hlslParseables.cpp

@@ -245,7 +245,6 @@ glslang::TString& AppendTypeName(glslang::TString& s, const char* argOrder, cons
     char order = *argOrder;
 
     if (UseHlslTypes) {
-        // TODO: handle sub-vec4 returns
         switch (type) {
         case '-': s += "void";                                            break;
         case 'F': s += "float";                                           break;
@@ -832,8 +831,6 @@ void TBuiltInParseablesHlsl::initialize(int /*version*/, EProfile /*profile*/, c
         { "GatherCmpAlpha",  /* O-4 */        "V4",    nullptr,   "%@,S,V,S,V,,,",  "FIU,s,F,,I,,,",  EShLangAll },
         { "GatherCmpAlpha",  /* O-4, status */"V4",    nullptr,   "%@,S,V,S,V,,,,S","FIU,s,F,,I,,,,U",EShLangAll },
 
-        // TODO: Cmp forms
-
         // Mark end of list, since we want to avoid a range-based for, as some compilers don't handle it yet.
         { nullptr,                            nullptr, nullptr,   nullptr,      nullptr,  0 },
     };