-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
JIT: Improve x86 unsigned to floating cast codegen #111595
Conversation
cc @dotnet/jit-contrib this is ready for review |
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.
CC. @dotnet/jit-contrib for secondary review
* main: System.Net.Http.WinHttpHandler.StartRequestAsync assertion failed (dotnet#109799) Keep test PDB in helix payload for native AOT (dotnet#111949) Build the RID-specific System.IO.Ports packages in the VMR (dotnet#112054) Always inline number conversions (dotnet#112061) Use Contains{Any} in Regex source generator (dotnet#112065) Update dependencies from https://github.com/dotnet/arcade build 20250130.5 (dotnet#112013) JIT: Transform single-reg args to FIELD_LIST in physical promotion (dotnet#111590) Ensure that math calls into the CRT are tracked as needing vzeroupper (dotnet#112011) Use double.ConvertToIntegerNative where safe to do in System.Random (dotnet#112046) JIT: Compute `fgCalledCount` after synthesis (dotnet#112041) Simplify boolean logic in `TimeZoneInfo` (dotnet#112062) JIT: Update type when return temp is freshly created (dotnet#111948) Remove unused build controls and simplify DotNetBuild.props (dotnet#111986) Fix case-insensitive JSON deserialization of enum member names (dotnet#112028) WasmAppBuilder: Remove double computation of a value (dotnet#112047) Disable LTCG for brotli and zlibng. (dotnet#111805) JIT: Improve x86 unsigned to floating cast codegen (dotnet#111595) simplify x86 special intrinsic imports (dotnet#111836) JIT: Try to retain entry weight during profile synthesis (dotnet#111971) Fix explicit offset of ByRefLike fields. (dotnet#111584)
@saucecontrol we're seeing some test failures (#112324, #112325, #112329) in our stress pipelines on x64 related to floating-point casts. Do those look related to this PR? |
Yeah, if it's only showing up under JitStressRegs, it's likely bad codegen from an existing bug this PR exposed by changing float->double->float casts to float->float. In which case, #112217 should resolve it. |
Actually, not so sure about that last one. This PR changed integral->floating casts, but those failures are on floating->integral, which I don't think has changed recently |
Thanks for confirming; I'll wait to see if #112217 resolves them... |
Fixes #77658
This improves codegen mostly for unsigned to floating types but catches a few other redundant conversions.
Adds support for using AVX-512
vcvtusi2s[sd]
for uint -> float/double (only ulong was handled previously) on both x64 and x86.Improves codegen for uint -> float conversions on x64 without AVX-512, removing the intermediate conversion to double.
Adds support for direct ulong -> float cast to the x64 SSE2 fallback, resolving a difference in behavior between hardware with AVX-512 vs without, and saving an extra
cvtsd2ss
instruction.Removes some redundant float -> double -> float casts.
SPMI Diffs
The only code size regressions are the insertion of
xorps
to clear the upper elements of the target reg for the AVX-512 unsigned conversion instructions. These were previously omitted but should have been there since the unsigned conversions have the same behavior as the signed (i.e. preserving/copying upper elements) and are subject to the same false dependency penalties.GCC emits the
xorps
for all conversions; Clang skips it for all conversions in simple examples but may emit it in more complex scenarios.https://godbolt.org/z/6aY7fdE3d