From 988cfcfc488fe71f6e13e01a3a3dfb962ced72e1 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 5 Apr 2024 20:59:11 -0700 Subject: [PATCH] Saturating floating point to integer conversions on Arm32 (#100682) * Saturating floating point to integer conversions on Arm32 Follow up on https://github.com/dotnet/runtime/pull/97529#discussion_r1534723237 * Fixes, cleanup --- src/coreclr/nativeaot/Runtime/MathHelpers.cpp | 25 ++----- src/coreclr/vm/jithelpers.cpp | 73 +++++++------------ .../out_of_range_fp_to_int_conversions.cpp | 33 --------- .../out_of_range_fp_to_int_conversions.cs | 48 +----------- 4 files changed, 37 insertions(+), 142 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/MathHelpers.cpp b/src/coreclr/nativeaot/Runtime/MathHelpers.cpp index 73a5aa924794d..6e3f4d73de8af 100644 --- a/src/coreclr/nativeaot/Runtime/MathHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/MathHelpers.cpp @@ -15,31 +15,20 @@ FCIMPL1_D(uint64_t, RhpDbl2ULng, double val) const double uint64_max_plus_1 = 4294967296.0 * 4294967296.0; return (val > 0) ? ((val >= uint64_max_plus_1) ? UINT64_MAX : (uint64_t)val) : 0; #else - const double two63 = 2147483648.0 * 4294967296.0; - uint64_t ret; - if (val < two63) - { - ret = (int64_t)(val); - } - else - { - // subtract 0x8000000000000000, do the convert then add it back again - ret = (int64_t)(val - two63) + I64(0x8000000000000000); - } - return ret; -#endif //HOST_X86 || HOST_AMD64 + return (uint64_t)val; +#endif } FCIMPLEND FCIMPL1_D(int64_t, RhpDbl2Lng, double val) { -#if defined(HOST_X86) || defined(HOST_AMD64) +#if defined(HOST_X86) || defined(HOST_AMD64) || defined(HOST_ARM) const double int64_min = -2147483648.0 * 4294967296.0; const double int64_max = 2147483648.0 * 4294967296.0; return (val != val) ? 0 : (val <= int64_min) ? INT64_MIN : (val >= int64_max) ? INT64_MAX : (int64_t)val; #else return (int64_t)val; -#endif //HOST_X86 || HOST_AMD64 +#endif } FCIMPLEND @@ -51,7 +40,7 @@ FCIMPL1_D(int32_t, RhpDbl2Int, double val) return (val != val) ? 0 : (val <= int32_min) ? INT32_MIN : (val >= int32_max_plus_1) ? INT32_MAX : (int32_t)val; #else return (int32_t)val; -#endif //HOST_X86 || HOST_AMD64 +#endif } FCIMPLEND @@ -62,7 +51,7 @@ FCIMPL1_D(uint32_t, RhpDbl2UInt, double val) return (val > 0) ? ((val >= uint_max) ? UINT32_MAX : (uint32_t)val) : 0; #else return (uint32_t)val; -#endif //HOST_X86 || HOST_AMD64 +#endif } FCIMPLEND @@ -358,4 +347,4 @@ FCIMPL2_FI(float, modff, float x, float* intptr) return std::modff(x, intptr); FCIMPLEND -#endif \ No newline at end of file +#endif diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 4614a89f403c4..3aae4a155fc77 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -491,51 +491,44 @@ HCIMPLEND #include /*********************************************************************/ -// -HCIMPL1_V(double, JIT_ULng2Dbl, UINT64 val) +HCIMPL1_V(double, JIT_ULng2Dbl, uint64_t val) { FCALL_CONTRACT; - - double conv = (double) ((INT64) val); - if (conv < 0) - conv += (4294967296.0 * 4294967296.0); // add 2^64 - _ASSERTE(conv >= 0); - return(conv); + return (double)val; } HCIMPLEND /*********************************************************************/ -// needed for ARM and RyuJIT-x86 -HCIMPL1_V(double, JIT_Lng2Dbl, INT64 val) +HCIMPL1_V(double, JIT_Lng2Dbl, int64_t val) { FCALL_CONTRACT; - return double(val); + return (double)val; } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(INT64, JIT_Dbl2Lng, double val) +HCIMPL1_V(int64_t, JIT_Dbl2Lng, double val) { FCALL_CONTRACT; -#if defined(TARGET_X86) || defined(TARGET_AMD64) +#if defined(TARGET_X86) || defined(TARGET_AMD64) || defined(TARGET_ARM) const double int64_min = -2147483648.0 * 4294967296.0; const double int64_max = 2147483648.0 * 4294967296.0; - return (val != val) ? 0 : (val <= int64_min) ? INT64_MIN : (val >= int64_max) ? INT64_MAX : (INT64)val; + return (val != val) ? 0 : (val <= int64_min) ? INT64_MIN : (val >= int64_max) ? INT64_MAX : (int64_t)val; #else - return((INT64)val); -#endif // TARGET_X86 || TARGET_AMD64 + return (int64_t)val; +#endif } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(UINT32, JIT_Dbl2UIntOvf, double val) +HCIMPL1_V(uint32_t, JIT_Dbl2UIntOvf, double val) { FCALL_CONTRACT; // Note that this expression also works properly for val = NaN case if (val > -1.0 && val < 4294967296.0) - return((UINT32)val); + return (uint32_t)val; FCThrow(kOverflowException); } @@ -549,14 +542,14 @@ HCIMPL1_V(int, JIT_Dbl2IntOvf, double val) const double two31 = 2147483648.0; // Note that this expression also works properly for val = NaN case if (val > -two31 - 1 && val < two31) - return((INT32)val); + return (int32_t)val; FCThrow(kOverflowException); } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(INT64, JIT_Dbl2LngOvf, double val) +HCIMPL1_V(int64_t, JIT_Dbl2LngOvf, double val) { FCALL_CONTRACT; @@ -565,77 +558,67 @@ HCIMPL1_V(INT64, JIT_Dbl2LngOvf, double val) // Note that this expression also works properly for val = NaN case // We need to compare with the very next double to two63. 0x402 is epsilon to get us there. if (val > -two63 - 0x402 && val < two63) - return((INT64)val); + return (int64_t)val; FCThrow(kOverflowException); } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(UINT64, JIT_Dbl2ULngOvf, double val) +HCIMPL1_V(uint64_t, JIT_Dbl2ULngOvf, double val) { FCALL_CONTRACT; const double two64 = 4294967296.0 * 4294967296.0; // Note that this expression also works properly for val = NaN case if (val > -1.0 && val < two64) - return (UINT64)val; + return (uint64_t)val; FCThrow(kOverflowException); } HCIMPLEND -HCIMPL1_V(UINT32, JIT_Dbl2UInt, double val) +HCIMPL1_V(uint32_t, JIT_Dbl2UInt, double val) { FCALL_CONTRACT; #if defined(TARGET_X86) || defined(TARGET_AMD64) const double uint_max = 4294967295.0; // Note that this expression also works properly for val = NaN case - return (val >= 0) ? ((val >= uint_max) ? UINT32_MAX : (UINT32)val) : 0; + return (val >= 0) ? ((val >= uint_max) ? UINT32_MAX : (uint32_t)val) : 0; #else - return((UINT32)val); -#endif //TARGET_X86 || TARGET_AMD64 + return (uint32_t)val; +#endif } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(INT32, JIT_Dbl2Int, double val) +HCIMPL1_V(int32_t, JIT_Dbl2Int, double val) { FCALL_CONTRACT; #if defined(TARGET_X86) || defined(TARGET_AMD64) const double int32_min = -2147483648.0; const double int32_max_plus_1 = 2147483648.0; - return (val != val) ? 0 : (val <= int32_min) ? INT32_MIN : (val >= int32_max_plus_1) ? INT32_MAX : (INT32)val; + return (val != val) ? 0 : (val <= int32_min) ? INT32_MIN : (val >= int32_max_plus_1) ? INT32_MAX : (int32_t)val; #else - return((INT32)val); -#endif // TARGET_X86 || TARGET_AMD64 + return (int32_t)val; +#endif } HCIMPLEND /*********************************************************************/ -HCIMPL1_V(UINT64, JIT_Dbl2ULng, double val) +HCIMPL1_V(uint64_t, JIT_Dbl2ULng, double val) { FCALL_CONTRACT; #if defined(TARGET_X86) || defined(TARGET_AMD64) const double uint64_max_plus_1 = 4294967296.0 * 4294967296.0; // Note that this expression also works properly for val = NaN case - return (val >= 0) ? ((val >= uint64_max_plus_1) ? UINT64_MAX : (UINT64)val) : 0; - + return (val >= 0) ? ((val >= uint64_max_plus_1) ? UINT64_MAX : (uint64_t)val) : 0; #else - const double two63 = 2147483648.0 * 4294967296.0; - UINT64 ret; - if (val < two63) { - ret = (INT64)(val); - } - else { - // subtract 0x8000000000000000, do the convert then add it back again - ret = (INT64)(val - two63) + I64(0x8000000000000000); - } - return ret; -#endif // TARGET_X86 || TARGET_AMD64 + return (uint64_t)val; +#endif } HCIMPLEND diff --git a/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cpp b/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cpp index a16932e8a78a4..65c5118060f39 100644 --- a/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cpp +++ b/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cpp @@ -17,7 +17,6 @@ typedef enum { CONVERT_SENTINEL, CONVERT_SATURATING, CONVERT_NATIVECOMPILERBEHAVIOR, - CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32, } FPtoIntegerConversionType; extern "C" DLLEXPORT int32_t ConvertDoubleToInt32(double x, FPtoIntegerConversionType t) @@ -32,7 +31,6 @@ extern "C" DLLEXPORT int32_t ConvertDoubleToInt32(double x, FPtoIntegerConversio case CONVERT_SENTINEL: return ((x != x) || (x < INT32_MIN) || (x > INT32_MAX)) ? INT32_MIN : (int32_t)x; - case CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: case CONVERT_SATURATING: return (x != x) ? 0 : (x < INT32_MIN) ? INT32_MIN : (x > INT32_MAX) ? INT32_MAX : (int32_t)x; case CONVERT_NATIVECOMPILERBEHAVIOR: // handled above, but add case to silence warning @@ -57,7 +55,6 @@ extern "C" DLLEXPORT uint32_t ConvertDoubleToUInt32(double x, FPtoIntegerConvers case CONVERT_SENTINEL: return ((x != x) || (x < 0) || (x > UINT32_MAX)) ? UINT32_MAX : (uint32_t)x; - case CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: case CONVERT_SATURATING: return ((x != x) || (x < 0)) ? 0 : (x > UINT32_MAX) ? UINT32_MAX : (uint32_t)x; case CONVERT_NATIVECOMPILERBEHAVIOR: // handled above, but add case to silence warning @@ -67,14 +64,6 @@ extern "C" DLLEXPORT uint32_t ConvertDoubleToUInt32(double x, FPtoIntegerConvers return 0; } -static uint64_t CppNativeArm32ConvertDoubleToUInt64(double y) -{ - const double uintmax_plus_1 = -2.0 * (double)INT32_MIN; - uint32_t hi32Bits = ConvertDoubleToUInt32(y / uintmax_plus_1, CONVERT_SATURATING); - uint32_t lo32Bits = ConvertDoubleToUInt32(y - (((double)hi32Bits) * uintmax_plus_1), CONVERT_SATURATING); - return (((uint64_t)hi32Bits) << 32) + lo32Bits; -} - extern "C" DLLEXPORT int64_t ConvertDoubleToInt64(double x, FPtoIntegerConversionType t) { if (t == CONVERT_NATIVECOMPILERBEHAVIOR) @@ -96,16 +85,6 @@ extern "C" DLLEXPORT int64_t ConvertDoubleToInt64(double x, FPtoIntegerConversio case CONVERT_SENTINEL: return ((x != x) || (x < INT64_MIN) || (x >= int64_max_plus_1)) ? INT64_MIN : (int64_t)x; - case CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: - if (x > 0) - { - return (int64_t)CppNativeArm32ConvertDoubleToUInt64(x); - } - else - { - return -(int64_t)CppNativeArm32ConvertDoubleToUInt64(-x); - } - case CONVERT_SATURATING: return (x != x) ? 0 : (x < INT64_MIN) ? INT64_MIN : (x >= int64_max_plus_1) ? INT64_MAX : (int64_t)x; case CONVERT_NATIVECOMPILERBEHAVIOR: // handled above, but add case to silence warning @@ -138,18 +117,6 @@ extern "C" DLLEXPORT uint64_t ConvertDoubleToUInt64(double x, FPtoIntegerConver case CONVERT_SATURATING: return ((x != x) || (x < 0)) ? 0 : (x >= uint64_max_plus_1) ? UINT64_MAX : (uint64_t)x; - case CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: - { - if (x < int64_max_plus_1) - { - return (uint64_t)ConvertDoubleToInt64(x, CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32); - } - else - { - return (uint64_t)ConvertDoubleToInt64(x - int64_max_plus_1, CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32) + (0x8000000000000000); - } - } - case CONVERT_NATIVECOMPILERBEHAVIOR: // handled above, but add case to silence warning return 0; } diff --git a/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cs b/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cs index e61078a0e0501..d78daddcd838a 100644 --- a/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cs +++ b/src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cs @@ -19,7 +19,6 @@ public enum FPtoIntegerConversionType CONVERT_SENTINEL, CONVERT_SATURATING, CONVERT_NATIVECOMPILERBEHAVIOR, - CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32, } public enum ConversionType @@ -91,7 +90,6 @@ public static int ConvertDoubleToInt32(double x, FPtoIntegerConversionType t) return (Double.IsNaN(x) || (x int.MaxValue)) ? int.MinValue: (int) x; case FPtoIntegerConversionType.CONVERT_SATURATING: - case FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: return Double.IsNaN(x) ? 0 : (x< int.MinValue) ? int.MinValue : (x > int.MaxValue) ? int.MaxValue : (int) x; } return 0; @@ -114,7 +112,6 @@ public static uint ConvertDoubleToUInt32(double x, FPtoIntegerConversionType t) return (Double.IsNaN(x) || (x < 0) || (x > uint.MaxValue)) ? uint.MaxValue : (uint)x; case FPtoIntegerConversionType.CONVERT_SATURATING: - case FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: return (Double.IsNaN(x) || (x < 0)) ? 0 : (x > uint.MaxValue) ? uint.MaxValue : (uint)x; } @@ -137,29 +134,11 @@ public static long ConvertDoubleToInt64(double x, FPtoIntegerConversionType t) case FPtoIntegerConversionType.CONVERT_SENTINEL: return (Double.IsNaN(x) || (x < long.MinValue) || (x >= llong_max_plus_1)) ? long.MinValue : (long)x; - case FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: - if (x > 0) - { - return (long)CppNativeArm32ConvertDoubleToUInt64(x); - } - else - { - return -(long)CppNativeArm32ConvertDoubleToUInt64(-x); - } - case FPtoIntegerConversionType.CONVERT_SATURATING: return Double.IsNaN(x) ? 0 : (x < long.MinValue) ? long.MinValue : (x >= llong_max_plus_1) ? long.MaxValue : (long)x; } return 0; - - static ulong CppNativeArm32ConvertDoubleToUInt64(double y) - { - const double uintmax_plus_1 = -2.0 * (double)int.MinValue; - uint hi32Bits = ConvertDoubleToUInt32(y / uintmax_plus_1, FPtoIntegerConversionType.CONVERT_SATURATING); - uint lo32Bits = ConvertDoubleToUInt32(y - (((double)hi32Bits) * uintmax_plus_1), FPtoIntegerConversionType.CONVERT_SATURATING); - return (((ulong)hi32Bits) << (int)32) + lo32Bits; - } } public static ulong ConvertDoubleToUInt64(double x, FPtoIntegerConversionType t) @@ -183,18 +162,6 @@ public static ulong ConvertDoubleToUInt64(double x, FPtoIntegerConversionType t) case FPtoIntegerConversionType.CONVERT_SATURATING: return (Double.IsNaN(x) || (x < 0)) ? 0 : (x >= ullong_max_plus_1) ? ulong.MaxValue : (ulong)x; - - case FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32: - { - if (x < two63) - { - return (ulong)ConvertDoubleToInt64(x, FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32); - } - else - { - return (ulong)ConvertDoubleToInt64(x - two63, FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32) + (0x8000000000000000); - } - } } return 0; @@ -261,7 +228,6 @@ static void TestBitValue(uint value, double? dblValNullable = null, FPtoIntegerC if (!tValue.HasValue) { - TestBitValue(value, dblVal, FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32); TestBitValue(value, dblVal, FPtoIntegerConversionType.CONVERT_BACKWARD_COMPATIBLE); TestBitValue(value, dblVal, FPtoIntegerConversionType.CONVERT_SATURATING); TestBitValue(value, dblVal, FPtoIntegerConversionType.CONVERT_SENTINEL); @@ -355,18 +321,8 @@ static void TestBitValue(uint value, double? dblValNullable = null, FPtoIntegerC [Fact] public static int TestEntryPoint() { - switch (RuntimeInformation.ProcessArchitecture) - { - case Architecture.Arm: - Program.ManagedConversionRule = FPtoIntegerConversionType.CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32; - break; - - case Architecture.X86: - case Architecture.X64: - case Architecture.Arm64: - Program.ManagedConversionRule = FPtoIntegerConversionType.CONVERT_SATURATING; - break; - } + Program.ManagedConversionRule = FPtoIntegerConversionType.CONVERT_SATURATING; + Console.WriteLine($"Expected managed float behavior is {Program.ManagedConversionRule} Execute with parameter to adjust"); Console.WriteLine("Specific test cases");