Skip to content

Add long/ulong->float cast helpers #114597

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

saucecontrol
Copy link
Member

@saucecontrol saucecontrol commented Apr 12, 2025

Resolves #106646

We currently have a few inconsistencies in the way long/ulong to float conversions are done in JIT.

Problem 1

On 64-bit platforms, long->float conversions have always been done directly with a native CPU instruction, while on 32-bit, long->float has always been morphed to long->double->float so that it could use CORINFO_HELP_LNG2DBL.

This can lead to different conversion results when the long->double conversion rounds in a different direction than long->float would. Ex: sharplab.

Similarly, ulong->float may yield a different result than ulong->double->float.

Problem 2

In CIL, there is no way to represent a conversion directly from unsigned to float or double. The IL instruction conv.r.un specifies unsigned conversion to IL type F which is of indeterminate precision. Consequently, managed language compilers emit a pair of instructions for these casts: conv.r.un; conv.r4 or conv.r.un; conv.r8.

The JIT importer has always treated conv.r.un as unsigned->double, so in the case of conv.r.un; conv.r8, the second (double->double) cast is skipped, and the intention of the managed compiler is preserved. conv.r.un; conv.r4, on the other hand, imports as unsigned->double->float.

As stated above, this could yield a different result than intended for ulong.

Problem 3

When Arm64 support was added to JIT, as an optimization to take advantage of the fact Arm64 has a direct ulong->float conversion instruction, recognition of the ulong->double->float pattern was added to JIT, and the intermediate cast was removed. This led to Arm64 having different cast behavior than all other platforms.

#84384 extended the same optimization to x64 in .NET 8, using AVX-512 instructions. This led to further fragmentation of behavior since the presence or absence of AVX-512 could change results.

#111595 unified the behavior on x64 by emulating the direct ulong->float cast using SSE instructions.

However, this still leaves problem 1 (32-bit still goes through CORINFO_HELP_ULNG2DBL) and introduces another: The intermediate double cast may have actually been intentional, and the current optimization removes it without knowing. i.e. conv.r.un; conv.r4 and conv.r.un; conv.r8; conv.r4 are treated the same.

The solution

This PR adds JIT helpers to perform long->float and ulong->float casts directly for 32-bit platforms.

It also modifies JIT to specifically look for the conv.r.un; conv.r4 sequence and import it as unsigned->float, and then calls the new helpers as appropriate.

Next steps

Because this requires a JIT-EE GUID change, I have broken the solution into two parts.

This part solves problems 1 and 2 by making sure all platforms can consistently recognize and perform the long/ulong->float casts directly.

I will address problem 3 in a followup PR, along with some more cleanup, optimization, and added tests. The current bad optimization also catches some redundant casts that will need to be handled differently, and it will be easier to see the impact of all those changes if the SPMI asmdiff jobs work.

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 12, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Apr 12, 2025
@AaronRobinsonMSFT AaronRobinsonMSFT added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Apr 12, 2025
@AaronRobinsonMSFT
Copy link
Member

/cc @dotnet/jit-contrib

Copy link
Contributor

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

@saucecontrol saucecontrol marked this pull request as ready for review April 13, 2025 01:11
@saucecontrol
Copy link
Member Author

saucecontrol commented Apr 13, 2025

jit-format is failing due to changes from #114525. cc @kunalspathak

(fixed in #114603)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JIT: Model Int64/UInt64 -> Single casts without intermediate cast to Double
3 participants