Updating a few BitConverter APIs to be intrinsic (#71567)

* Updating a few BitConverter APIs to be intrinsic * Reacting to PR feedback * Update src/coreclr/jit/valuenum.cpp Co-authored-by: SingleAccretion <[email protected]> * Update src/coreclr/jit/gentree.h Co-authored-by: SingleAccretion <[email protected]> * Apply formatting patch * Ensure DoubleToInt64Bits and Int64BitsToDouble intrinsics only pop when appropriate * Fix VNForBitCast to not handle exception set logic since its always a normal value now * Applying formatting patch Co-authored-by: SingleAccretion <[email protected]>
dotnet · Jul 7, 2022 · 79d5e09 · 79d5e09
1 parent 8b19573
commit 79d5e09
Show file tree

Hide file tree

Showing 11 changed files with 306 additions and 68 deletions.
diff --git a/src/coreclr/jit/codegenxarch.cpp b/src/coreclr/jit/codegenxarch.cpp
@@ -490,19 +490,24 @@ void CodeGen::genSetRegToConst(regNumber targetReg, var_types targetType, GenTre
 
         case GT_CNS_DBL:
         {
-            emitter* emit       = GetEmitter();
-            emitAttr size       = emitTypeSize(targetType);
-            double   constValue = tree->AsDblCon()->gtDconVal;
+            emitter* emit = GetEmitter();
+            emitAttr size = emitTypeSize(targetType);
 
-            // Make sure we use "xorps reg, reg" only for +ve zero constant (0.0) and not for -ve zero (-0.0)
-            if (*(__int64*)&constValue == 0)
+            if (tree->IsFloatPositiveZero())
             {
-                // A faster/smaller way to generate 0
+                // A faster/smaller way to generate Zero
                 emit->emitIns_R_R(INS_xorps, size, targetReg, targetReg);
             }
+            else if (tree->IsFloatAllBitsSet())
+            {
+                // A faster/smaller way to generate AllBitsSet
+                emit->emitIns_R_R(INS_pcmpeqd, size, targetReg, targetReg);
+            }
             else
             {
-                CORINFO_FIELD_HANDLE hnd = emit->emitFltOrDblConst(constValue, size);
+                double               cns = tree->AsDblCon()->gtDconVal;
+                CORINFO_FIELD_HANDLE hnd = emit->emitFltOrDblConst(cns, size);
+
                 emit->emitIns_R_C(ins_Load(targetType), size, targetReg, hnd, 0);
             }
         }

diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h
@@ -4872,6 +4872,9 @@ class Compiler
     // Does value-numbering for a cast tree.
     void fgValueNumberCastTree(GenTree* tree);
 
+    // Does value-numbering for a bitcast tree.
+    void fgValueNumberBitCast(GenTree* tree);
+
     // Does value-numbering for an intrinsic tree.
     void fgValueNumberIntrinsic(GenTree* tree);
 

diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp
@@ -4567,9 +4567,10 @@ unsigned Compiler::gtSetEvalOrder(GenTree* tree)
             {
                 level = 0;
 #if defined(TARGET_XARCH)
-                if (tree->IsFloatPositiveZero())
+                if (tree->IsFloatPositiveZero() || tree->IsFloatAllBitsSet())
                 {
-                    // We generate `xorp* tgtReg, tgtReg` which is 3-5 bytes
+                    // We generate `xorp* tgtReg, tgtReg` for PositiveZero and
+                    // `pcmpeqd tgtReg, tgtReg` for AllBitsSet which is 3-5 bytes
                     // but which can be elided by the instruction decoder.
 
                     costEx = 1;

diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h
@@ -1828,6 +1828,7 @@ struct GenTree
 #endif // DEBUG
 
     inline bool IsIntegralConst(ssize_t constVal) const;
+    inline bool IsFloatAllBitsSet() const;
     inline bool IsFloatNaN() const;
     inline bool IsFloatPositiveZero() const;
     inline bool IsFloatNegativeZero() const;
@@ -8458,6 +8459,33 @@ inline bool GenTree::IsIntegralConst(ssize_t constVal) const
     return false;
 }
 
+//-------------------------------------------------------------------
+// IsFloatAllBitsSet: returns true if this is exactly a const float value representing AllBitsSet.
+//
+// Returns:
+//     True if this represents a const floating-point value representing AllBitsSet.
+//     Will return false otherwise.
+//
+inline bool GenTree::IsFloatAllBitsSet() const
+{
+    if (IsCnsFltOrDbl())
+    {
+        double constValue = AsDblCon()->gtDconVal;
+
+        if (TypeIs(TYP_FLOAT))
+        {
+            return FloatingPointUtils::isAllBitsSet(static_cast<float>(constValue));
+        }
+        else
+        {
+            assert(TypeIs(TYP_DOUBLE));
+            return FloatingPointUtils::isAllBitsSet(constValue);
+        }
+    }
+
+    return false;
+}
+
 //-------------------------------------------------------------------
 // IsFloatNaN: returns true if this is exactly a const float value of NaN
 //
@@ -8509,7 +8537,7 @@ inline bool GenTree::IsFloatPositiveZero() const
         // but it is easier to parse out
         // rather than using !IsCnsNonZeroFltOrDbl.
         double constValue = AsDblCon()->gtDconVal;
-        return *(__int64*)&constValue == 0;
+        return FloatingPointUtils::isPositiveZero(constValue);
     }
 
     return false;

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
@@ -4880,6 +4880,96 @@ GenTree* Compiler::impIntrinsic(GenTree*                newobjThis,
                 break;
             }
 
+            case NI_System_BitConverter_DoubleToInt64Bits:
+            {
+                GenTree* op1 = impStackTop().val;
+                assert(varTypeIsFloating(op1));
+
+                if (op1->IsCnsFltOrDbl())
+                {
+                    impPopStack();
+
+                    double f64Cns = op1->AsDblCon()->gtDconVal;
+                    retNode       = gtNewLconNode(*reinterpret_cast<int64_t*>(&f64Cns));
+                }
+#if TARGET_64BIT
+                else
+                {
+                    // TODO-Cleanup: We should support this on 32-bit but it requires decomposition work
+                    impPopStack();
+
+                    if (op1->TypeGet() != TYP_DOUBLE)
+                    {
+                        op1 = gtNewCastNode(TYP_DOUBLE, op1, false, TYP_DOUBLE);
+                    }
+                    retNode = gtNewBitCastNode(TYP_LONG, op1);
+                }
+#endif
+                break;
+            }
+
+            case NI_System_BitConverter_Int32BitsToSingle:
+            {
+                GenTree* op1 = impPopStack().val;
+                assert(varTypeIsInt(op1));
+
+                if (op1->IsIntegralConst())
+                {
+                    int32_t i32Cns = (int32_t)op1->AsIntConCommon()->IconValue();
+                    retNode        = gtNewDconNode(*reinterpret_cast<float*>(&i32Cns), TYP_FLOAT);
+                }
+                else
+                {
+                    retNode = gtNewBitCastNode(TYP_FLOAT, op1);
+                }
+                break;
+            }
+
+            case NI_System_BitConverter_Int64BitsToDouble:
+            {
+                GenTree* op1 = impStackTop().val;
+                assert(varTypeIsLong(op1));
+
+                if (op1->IsIntegralConst())
+                {
+                    impPopStack();
+
+                    int64_t i64Cns = op1->AsIntConCommon()->LngValue();
+                    retNode        = gtNewDconNode(*reinterpret_cast<double*>(&i64Cns));
+                }
+#if TARGET_64BIT
+                else
+                {
+                    // TODO-Cleanup: We should support this on 32-bit but it requires decomposition work
+                    impPopStack();
+
+                    retNode = gtNewBitCastNode(TYP_DOUBLE, op1);
+                }
+#endif
+                break;
+            }
+
+            case NI_System_BitConverter_SingleToInt32Bits:
+            {
+                GenTree* op1 = impPopStack().val;
+                assert(varTypeIsFloating(op1));
+
+                if (op1->IsCnsFltOrDbl())
+                {
+                    float f32Cns = (float)op1->AsDblCon()->gtDconVal;
+                    retNode      = gtNewIconNode(*reinterpret_cast<int32_t*>(&f32Cns));
+                }
+                else
+                {
+                    if (op1->TypeGet() != TYP_FLOAT)
+                    {
+                        op1 = gtNewCastNode(TYP_FLOAT, op1, false, TYP_FLOAT);
+                    }
+                    retNode = gtNewBitCastNode(TYP_INT, op1);
+                }
+                break;
+            }
+
             default:
                 break;
         }
@@ -5518,6 +5608,53 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
                 result = NI_System_Activator_DefaultConstructorOf;
             }
         }
+        else if (strcmp(className, "BitConverter") == 0)
+        {
+            if (methodName[0] == 'D')
+            {
+                if (strcmp(methodName, "DoubleToInt64Bits") == 0)
+                {
+                    result = NI_System_BitConverter_DoubleToInt64Bits;
+                }
+                else if (strcmp(methodName, "DoubleToUInt64Bits") == 0)
+                {
+                    result = NI_System_BitConverter_DoubleToInt64Bits;
+                }
+            }
+            else if (methodName[0] == 'I')
+            {
+                if (strcmp(methodName, "Int32BitsToSingle") == 0)
+                {
+                    result = NI_System_BitConverter_Int32BitsToSingle;
+                }
+                else if (strcmp(methodName, "Int64BitsToDouble") == 0)
+                {
+                    result = NI_System_BitConverter_Int64BitsToDouble;
+                }
+            }
+            else if (methodName[0] == 'S')
+            {
+                if (strcmp(methodName, "SingleToInt32Bits") == 0)
+                {
+                    result = NI_System_BitConverter_SingleToInt32Bits;
+                }
+                else if (strcmp(methodName, "SingleToUInt32Bits") == 0)
+                {
+                    result = NI_System_BitConverter_SingleToInt32Bits;
+                }
+            }
+            else if (methodName[0] == 'U')
+            {
+                if (strcmp(methodName, "UInt32BitsToSingle") == 0)
+                {
+                    result = NI_System_BitConverter_Int32BitsToSingle;
+                }
+                else if (strcmp(methodName, "UInt64BitsToDouble") == 0)
+                {
+                    result = NI_System_BitConverter_Int64BitsToDouble;
+                }
+            }
+        }
         else if (strcmp(className, "Math") == 0 || strcmp(className, "MathF") == 0)
         {
             if (strcmp(methodName, "Abs") == 0)

diff --git a/src/coreclr/jit/namedintrinsiclist.h b/src/coreclr/jit/namedintrinsiclist.h
@@ -15,6 +15,11 @@ enum NamedIntrinsic : unsigned short
 
     NI_System_Enum_HasFlag,
 
+    NI_System_BitConverter_DoubleToInt64Bits,
+    NI_System_BitConverter_Int32BitsToSingle,
+    NI_System_BitConverter_Int64BitsToDouble,
+    NI_System_BitConverter_SingleToInt32Bits,
+
     NI_SYSTEM_MATH_START,
     NI_System_Math_Abs,
     NI_System_Math_Acos,

diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp
@@ -2232,6 +2232,38 @@ bool FloatingPointUtils::hasPreciseReciprocal(float x)
     return mantissa == 0 && exponent != 0 && exponent != 127;
 }
 
+//------------------------------------------------------------------------
+// isAllBitsSet: Determines whether the specified value is AllBitsSet
+//
+// Arguments:
+//    val - value to check for AllBitsSet
+//
+// Return Value:
+//    True if val is AllBitsSet
+//
+
+bool FloatingPointUtils::isAllBitsSet(float val)
+{
+    UINT32 bits = *reinterpret_cast<UINT32*>(&val);
+    return bits == 0xFFFFFFFFU;
+}
+
+//------------------------------------------------------------------------
+// isAllBitsSet: Determines whether the specified value is AllBitsSet
+//
+// Arguments:
+//    val - value to check for AllBitsSet
+//
+// Return Value:
+//    True if val is AllBitsSet
+//
+
+bool FloatingPointUtils::isAllBitsSet(double val)
+{
+    UINT64 bits = *reinterpret_cast<UINT64*>(&val);
+    return bits == 0xFFFFFFFFFFFFFFFFULL;
+}
+
 //------------------------------------------------------------------------
 // isNegative: Determines whether the specified value is negative
 //
@@ -2310,6 +2342,22 @@ bool FloatingPointUtils::isNegativeZero(double val)
     return bits == 0x8000000000000000ULL;
 }
 
+//------------------------------------------------------------------------
+// isPositiveZero: Determines whether the specified value is positive zero (+0.0)
+//
+// Arguments:
+//    val - value to check for (+0.0)
+//
+// Return Value:
+//    True if val is (+0.0)
+//
+
+bool FloatingPointUtils::isPositiveZero(double val)
+{
+    UINT64 bits = *reinterpret_cast<UINT64*>(&val);
+    return bits == 0x0000000000000000ULL;
+}
+
 //------------------------------------------------------------------------
 // maximum: This matches the IEEE 754:2019 `maximum` function
 //    It propagates NaN inputs back to the caller and

diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h
@@ -703,6 +703,10 @@ class FloatingPointUtils
 
     static float infinite_float();
 
+    static bool isAllBitsSet(float val);
+
+    static bool isAllBitsSet(double val);
+
     static bool isNegative(float val);
 
     static bool isNegative(double val);
@@ -713,6 +717,8 @@ class FloatingPointUtils
 
     static bool isNegativeZero(double val);
 
+    static bool isPositiveZero(double val);
+
     static double maximum(double val1, double val2);
 
     static float maximum(float val1, float val2);