Use genTypeCanRepresentValue only with small types
authorMike Danes <onemihaid@hotmail.com>
Mon, 29 Jan 2018 16:31:36 +0000 (18:31 +0200)
committerMike Danes <onemihaid@hotmail.com>
Mon, 29 Jan 2018 16:31:36 +0000 (18:31 +0200)
It's not needed and it's problematic to make it work for other types. In the JIT constant values are represented using `ssize_t` and you can't distinguish between `-1` and `UINT64_MAX`.

src/jit/codegenxarch.cpp
src/jit/compiler.hpp
src/jit/lower.cpp

index 7ff2ca7..32eb482 100644 (file)
@@ -6219,7 +6219,7 @@ void CodeGen::genCompareInt(GenTreePtr treeNode)
 #ifdef _TARGET_X86_
             (!op1->isUsedFromReg() || isByteReg(op1->gtRegNum)) &&
 #endif
-            (op2->IsCnsIntOrI() && genTypeCanRepresentValue(TYP_UBYTE, op2->AsIntCon()->IconValue())))
+            (op2->IsCnsIntOrI() && genSmallTypeCanRepresentValue(TYP_UBYTE, op2->AsIntCon()->IconValue())))
         {
             type = TYP_UBYTE;
         }
@@ -6266,8 +6266,9 @@ void CodeGen::genCompareInt(GenTreePtr treeNode)
         assert((genTypeSize(op1Type) >= genTypeSize(type)) || !op1->isUsedFromMemory());
         // If op2 is smaller then it cannot be in memory, we're probably missing a cast
         assert((genTypeSize(op2Type) >= genTypeSize(type)) || !op2->isUsedFromMemory());
-        // If op2 is a constant then it should fit in the common type
-        assert(!op2->IsCnsIntOrI() || genTypeCanRepresentValue(type, op2->AsIntCon()->IconValue()));
+        // If we ended up with a small type and op2 is a constant then make sure we don't lose constant bits
+        assert(!op2->IsCnsIntOrI() || !varTypeIsSmall(type) ||
+               genSmallTypeCanRepresentValue(type, op2->AsIntCon()->IconValue()));
     }
 
     // The type cannot be larger than the machine word size
index 7e1619b..edfdd2e 100644 (file)
@@ -514,7 +514,7 @@ inline regNumber genRegNumFromMask(regMaskTP mask)
 }
 
 //------------------------------------------------------------------------------
-// genTypeCanRepresentValue: Checks if a value can be represented by a given type.
+// genSmallTypeCanRepresentValue: Checks if a value can be represented by a given small type.
 //
 // Arguments:
 //    value - the value to check
@@ -522,13 +522,8 @@ inline regNumber genRegNumFromMask(regMaskTP mask)
 //
 // Return Value:
 //    True if the value is representable, false otherwise.
-//
-// Notes:
-//    If the type is not integral or ref like (ref/byref/array) then false is
-//    always returned.
 
-template <typename TValue>
-inline bool genTypeCanRepresentValue(var_types type, TValue value)
+inline bool genSmallTypeCanRepresentValue(var_types type, ssize_t value)
 {
     switch (type)
     {
@@ -541,19 +536,8 @@ inline bool genTypeCanRepresentValue(var_types type, TValue value)
             return FitsIn<UINT16>(value);
         case TYP_SHORT:
             return FitsIn<INT16>(value);
-        case TYP_UINT:
-            return FitsIn<UINT32>(value);
-        case TYP_INT:
-            return FitsIn<INT32>(value);
-        case TYP_ULONG:
-            return FitsIn<UINT64>(value);
-        case TYP_LONG:
-            return FitsIn<INT64>(value);
-        case TYP_REF:
-        case TYP_BYREF:
-            return FitsIn<UINT_PTR>(value);
         default:
-            return false;
+            unreached();
     }
 }
 
index b007fd7..9fc055e 100644 (file)
@@ -2666,7 +2666,7 @@ GenTree* Lowering::OptimizeConstCompare(GenTree* cmp)
     ssize_t        op2Value = op2->IconValue();
 
 #ifdef _TARGET_XARCH_
-    if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && genTypeCanRepresentValue(op1Type, op2Value))
+    if (IsContainableMemoryOp(op1) && varTypeIsSmall(op1Type) && genSmallTypeCanRepresentValue(op1Type, op2Value))
     {
         //
         // If op1's type is small then try to narrow op2 so it has the same type as op1.