Match jit behavior when converting float/double to u1/u2

[BrzVlad]

Previously it converted first to uint32_t, which was doing a saturating conversion. Example:

Before
(ushort)-4567.0f = (ushort)(uint)-4567.0f = (ushort)0 = 0

After
(ushort)-4567.0f = (ushort)(int)-4567.0f = (ushort)-4567 = 60969

Fixes System.Runtime.InteropServices.Tests.NFloatTests.NFloatTo* tests on interpreter

[dotnet-policy-service]

Tagging subscribers to this area: @JulieLeeMSFT, @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

[BrzVlad]

I'm not 100% sure about this but documentation from https://learn.microsoft.com/en-us/dotnet/core/compatibility/jit/9.0/fp-to-integer seems to suggest that saturating happens only for float -> int/uint conversions, not for the smaller integer types. Also per discussion from #116823

[Copilot]

Pull request overview

This PR adjusts CoreCLR interpreter float/double → unsigned 8/16-bit conversion opcodes to better align with JIT casting behavior (notably for negative inputs), fixing failures in existing System.Runtime.InteropServices NFloat conversion tests when run under the interpreter.

Changes:

• Update INTOP_CONV_U1_R4/R8 to convert via a signed 32-bit intermediate (int32_t) instead of uint32_t.
• Update INTOP_CONV_U2_R4/R8 to convert via a signed 32-bit intermediate (int32_t) instead of uint32_t.

[jkotas]

I'm not 100% sure about this but documentation from https://learn.microsoft.com/en-us/dotnet/core/compatibility/jit/9.0/fp-to-integer seems to suggest that saturating happens only for float -> int/uint conversions,

The idea was to do the saturating in all cases, but we have missed a few #116823 (comment)

[jkotas]

You may want to go over places that reference this issue number https://github.com/search?q=repo%3Adotnet%2Fruntime%20116823&type=code . I think the interpreter smoke tests may need fixing, and the two tests under libraries can be enabled now.

[BrzVlad]

I tried running those but they were still failing. Am I understanding correctly that we plan to change the behavior also for the small integer conversion in the future ? So at some point we would revert this PR back.

[jkotas]

I tried running those but they were still failing.

If this change is not replicating the current JIT behavior completely, it may be better to just disable the failing tests with [ActiveIssue("https://github.com/dotnet/runtime/issues/116823", typeof(PlatformDetection), nameof(PlatformDetection.IsCoreClrInterpreter))].

[BrzVlad]

Let me be more specific, consider this code:

double d = double.MinValue;
Console.WriteLine ((short)d);
Console.WriteLine (double.ConvertToInteger<short>(d));

The (short)d conversion first does the saturating int32 conversion and then the unsaturated conversion to short. ConvertToInteger does a saturating conversion directly to short in

runtime/src/libraries/System.Private.CoreLib/src/System/Int16.cs

Line 957 in c196cc0

double actualValue = (double)(object)value;

.

jit prints 0 and 0 (jit discards the managed implementation and intrinsifies with its own calculation)
interp prints 0 and -32768

This PR doesn't change anything in this scenario. Arguably, it is very weird that these conversions have different results, but it seems like this is currently by design ?

In conclusion, I'm starting to suspect that all conversions are meant to be saturating, so both interp and jit might be wrong, while the test for the other issues (ex. System.Tests.DoubleTests_GenericMath.ConvertToIntegerTest) is definitely wrong as it should expect -32768 rather than 0.

runtime/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/DoubleTests.GenericMath.cs

Line 354 in c196cc0

Assert.Equal(0, FloatingPointHelper<double>.ConvertToInteger<short>(double.MinValue));