Skip to content

Commit

Permalink
Saturating floating point to integer conversions on Arm32
Browse files Browse the repository at this point in the history 
Follow up on dotnet#97529 (comment)
  • Loading branch information
@jkotas
jkotas committed Apr 5, 2024
1 parent f91b911 commit 77af230
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 102 deletions.
15 changes: 2 additions & 13 deletions src/coreclr/nativeaot/Runtime/MathHelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -358,4 +347,4 @@ FCIMPL2_FI(float, modff, float x, float* intptr)
return std::modff(x, intptr);
FCIMPLEND

#endif
#endif
11 changes: 1 addition & 10 deletions src/coreclr/vm/jithelpers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
33 changes: 0 additions & 33 deletions src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
48 changes: 2 additions & 46 deletions src/tests/JIT/Directed/Convert/out_of_range_fp_to_int_conversions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ public enum FPtoIntegerConversionType
CONVERT_SENTINEL,
CONVERT_SATURATING,
CONVERT_NATIVECOMPILERBEHAVIOR,
CONVERT_MANAGED_BACKWARD_COMPATIBLE_ARM32,
}

public enum ConversionType
Expand Down Expand Up @@ -91,7 +90,6 @@ public static int ConvertDoubleToInt32(double x, FPtoIntegerConversionType t)
return (Double.IsNaN(x) || (x<int.MinValue) || (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;
Expand All @@ -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;
}

Expand All @@ -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)
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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");

Expand Down

0 comments on commit 77af230

Please sign in to comment.