-
Notifications
You must be signed in to change notification settings - Fork 805
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
uint64 literals generate suboptimal IL code #8683
Comments
JIT ASM for the F# and C# is identical on CoreCLR: F#: https://sharplab.io/#v2:DYLgZgzgNALiCGEC2AfYBTGACAlgOwGMsAPLAXhKwGosBGAVQBkg But it may not be true for desktop .NET. |
I know about the JITted code being identical. The problem is that F#'s IL code is not as compact as it is with C#. And as I said before, it might affect how the JIT performs inlining. |
Compactness of IL isn't necessarily a problem, which is also why optimization is difficult here. If the JIT optimizes it all the same, we'd need some evidence that this affects JIT inlining to say it's really a problem. As it stands, I think the best rationale for making a change here is consistency with C#. |
I agree. What source files are responsible for that? I want to try to fix it myself. |
Awesome! A good place to start would probably be in ILXGen, perhaps here: https://github.com/dotnet/fsharp/blob/master/src/fsharp/IlxGen.fs#L2433 Typically ILXGen is where the "translate F# to IL" happens, though some lower-level stuff can occur elsewhere. |
Repro steps
Consider this function:
Now look at the generated IL code:
Expected behavior
This is what C# generates. Generally, it should emit
ldc.i4.#
/ldc.i4.s
/ldc.i4
with aconv
, and only if the number is even bigger should it emitldc.i8
.Actual behavior
The function's IL code takes 12 bytes, instead of 5, as expected. This might interfere with the JIT's decision to inline small functions.
Known workarounds
Related information
This happens only with
uint64
literals. Literals of all other integral types (including signedint64
) emit optimal code.The text was updated successfully, but these errors were encountered: