From 77af23088674ffccf750035953b0b4b62be546ab Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 5 Apr 2024 08:10:00 -0700 Subject: [PATCH] Saturating floating point to integer conversions on Arm32 Follow up on https://github.com/dotnet/runtime/pull/97529#discussion_r1534723237 --- src/coreclr/nativeaot/Runtime/MathHelpers.cpp | 15 +----- src/coreclr/vm/jithelpers.cpp | 11 +---- .../out_of_range_fp_to_int_conversions.cpp | 33 ------------- .../out_of_range_fp_to_int_conversions.cs | 48 +------------------ 4 files changed, 5 insertions(+), 102 deletions(-) diff --git a/src/coreclr/nativeaot/Runtime/MathHelpers.cpp b/src/coreclr/nativeaot/Runtime/MathHelpers.cpp index 73a5aa924794dd..f80abfc3df2e34 100644 --- a/src/coreclr/nativeaot/Runtime/MathHelpers.cpp +++ b/src/coreclr/nativeaot/Runtime/MathHelpers.cpp @@ -15,18 +15,7 @@ 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; + return (uint64_t)val; #endif //HOST_X86 || HOST_AMD64 } 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 4614a89f403c49..9fee6a9485376a 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -625,16 +625,7 @@ HCIMPL1_V(UINT64, JIT_Dbl2ULng, double val) return (val >= 0) ? ((val >= uint64_max_plus_1) ? UINT64_MAX : (UINT64)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; + return((UINT64)val); #endif // TARGET_X86 || TARGET_AMD64 } 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 a16932e8a78a45..65c5118060f39f 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 e61078a0e05016..d78daddcd838a3 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");