-
-
Notifications
You must be signed in to change notification settings - Fork 147
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
make int->float conversion a transmute #105
Conversation
4673fc2
to
f847070
Compare
Changed to transmute based on feedback on upstream Rust change. Note that this basically gives us a cross-platform guarantee of if you feed in a NaN from one end, you get "some" NaN on the other end. The bits will match, but signaling-ness may toggle. |
This is the mirror commit to rust-lang/rust#46012
@gankro Thanks! I think you need to update (or remove?) the
Could you say more about this? I don't think I understand. If the bits don't change, how does signaling-ness change? (Please keep in mind that my knowledge of floating point is quite simplistic.) |
Basically until 2008, IEEE didn’t spec how to determine signalingness. Most platforms ended up picking the one that was later spec’d but e.g. MIPS picked the opposite interpretation of the signaling bit. So a signaling NaN on x86 is quiet on MIPS and vice-versa. |
This leaves any protocol which wishes to communicate between two systems with different signaling interpretations with a choice: either guarantee the NaN payload is preserved, or guarantee the signalingness is preserved. For various reasons signaling is a bit of a mess, so it’s hard for it to be useful, making my preference preserving the NaN payload (which is also the easiest impl: transmute on both sides). |
If you prefer the other option, the "obviously correct" canonicalization is still the 2008 spec, which still makes almost every platform read/write with transmute. MIPS and the few other "rogue" platforms would need to check if the value is NaN and flip a bit on read/write (similar to the current reading code). |
@gankro Ah thanks for explaining that, I finally understand. Keeping it as a transmute is fine with me! |
(But this does still need to remove the snan test!) |
Yeah sorry I'm waiting to get the r+ on the upstream patch to finish this
up.
…On Thu, Nov 16, 2017 at 4:26 PM, Andrew Gallant ***@***.***> wrote:
(But this does still need to remove the snan test!)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#105 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABFY4GpjbUciuVKXaVYz5BA8VlLuFLcZks5s3KiGgaJpZM4Qfhz3>
.
|
Accepted upstream, pushed test removal. |
f847070
to
a8abb86
Compare
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.
Thanks @gankro! I have a couple of fixups to make and then I'll push out byteorder 1.2
.
This is the mirror commit to rust-lang/rust#46012