-
Notifications
You must be signed in to change notification settings - Fork 5.1k
[release/9.0] JIT: Cast UInt64 to Single directly during const folding #106720
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
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
@tannergooding PTAL, thank you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approved. we can merge when ready
/ba-g Build Analysis is red, despite all issues being known |
@jeffschwMSFT @carlossanlop could you merge this when you have a moment? I'm not authorized to merge to |
@amanasifkhalid can you update the branch as it is out of date? |
@jeffschwMSFT @carlossanlop does this PR need anything else, or can it be merged? |
looks like it still needs a branch update. with that it can be merged |
Done |
Backport of #106419 to release/9.0
/cc @amanasifkhalid
Customer Impact
#106338 exposed different cast behavior between
ulong
andfloat
on ARM64 (and x64 with AVX-512) in Debug and Release builds. In the unoptimized case, we emit code to do the cast, and ARM64 supports casting directly from 64-bit integers tofloat
without an intermediate cast todouble
. When optimizing, we evaluate the cast at compile-time, but the JIT's constant folding logic does the cast in two steps (i.e.ulong -> double -> float
), which as demonstrated by the above issue, can result in subtle rounding differences. Our toolchain's compilers (MSVC/Clang/GCC/etc.) all emit the correct instruction sequence forulong -> float
casts, so the simplest fix for Debug/Release diffs is to adjust the constant-folding logic to cast directly tofloat
.While this fixes Debug/Release diffs, it's worth noting that the JIT still performs two-step casts on some platforms (x86, x64 without AVX-512, among others). This has to do partly with the fact that Roslyn emits a two-opcode IL sequence for
ulong -> float
casts, and RyuJIT imports this IL as aulong -> double -> float
cast; later on, RyuJIT may or may not decide to morph this into aulong -> float
cast, depending on the target architecture. We plan to fix this for all platforms such that an intermediate cast is never introduced (and if we import a two-step cast, we don't incorrectly morph it into one cast), but this work would likely be too disruptive to include in .NET 9 at this point. We're soliciting discussion on a solution for this in #106646.Regression
Testing
This issue was exposed when one of our fuzz testers generated a case that exercises subtle floating-point semantics. My fix includes a regression test to check the casting behavior we currently expect across our supported platforms. Once #106646 is fixed, we can expect the casting behavior this test demonstrates to be unified across platforms.
Risk
Low. This fix is quite surgical in removing Debug/Release diffs on platforms RyuJIT currently supports
ulong -> float
casts for.