-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Use HW-intrinsics in BitConverter for double <-> long / float <-> int #33476
Conversation
…nt to emit movd instead of using the stack
It fixes #11413 I guess (if it wasn't expected to be implemented in jit instead) |
Any concerns with this affecting AOT? I think there were some recent improvements re: how AOT and SSE2 work together. But would be good to get confirmation that this won't negatively impact anything. |
There shouldn't be. This is SSE/SSE2 so it should be part of the baseline instruction set. cc. @dotnet/jit-contrib should also review and may be able to provide input on if this is something better handled in JIT (maybe reinterpret casting for Edit: If it can/should be in the JIT, but that work is unlikely to be done for .NET 5, then it might be worthwhile to take the change and to track removing it when the JIT can do this more generally. |
ping @dotnet/jit-contrib |
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.
Assuming no JIT or AOT issues
I don't have a problem with taking this improvement. It might be nice to add a comment explaining that this is working around the fact that the JIT doesn't produce the code that one might expect (and possibly leaving #11413 open to address that workaround). |
Added in d271fd3 |
Thanks for the contribution @gfoidl! |
... to emit
movd
instead of using the stack on hardware that hasSSE2
.Ideally the JIT would emit code like this, so this workaround isn't needed.
(But my knowledge of JIT-programming is too limited to make the proper change over there.)
Cf. #12733 (comment) and #33057 (comment)
Code used for generating the asm-dumps
asm before
asm after
/cc: @tannergooding