-
Notifications
You must be signed in to change notification settings - Fork 699
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
Invalid NaN literal #28
Comments
spec repo gives a clearer error (for nan:0x7ff8000000000000):
basically, |
Hmm, that's a problem: it's unspecified by IEEE-754 what the NaN encoding means. Encoding the payload makes more sense honestly, but then how do we encode the sign? We could just fix binaryen, but I'd like us to figure out how exactly. |
When looking at parsing these it did occur that it would be convenient and clear in some contexts to be able to just specify the raw bits. Is there a good reason this is not allowed. For example, change nan:... to raw:... or bits:... |
Hexadecimal floating-point is the way to go for this... except for NaNs. |
@jfbastien |
Right, what I mean is that the hexadecimal floating-point specification handles all values precisely, but NaNs aren't a value and they're not handled by hexfloat. |
A producer might not even want to mess with hexfloat, just reliably emit the raw bit, and could do so using the same method as for nan:... It would not seem to complicate the consumer, perhaps even avoid a few tests. |
@JSStats discussed here: WebAssembly/design#292 |
To address one specific detail here: IEEE 754 defines unary negation as a purely bitwise operation with behavior defined even on NaNs: it just sets the sign bit. Consequently, it's reasonably consistent to use |
@sunfishcode ooh cute! I like :) |
As discussed with @binji and @sunfish in WebAssembly/wabt#28
I'll call this bug fixed by the binaryen PR. |
I'm testing a program which encounters the following sexpr-wasm error:
This was generated by binaryen's s2wasm, and seems to be in line with the spec repo. @binji could you confirm?
/cc @kripken @rossberg-chromium
The text was updated successfully, but these errors were encountered: