-
Notifications
You must be signed in to change notification settings - Fork 455
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
Implement accurate float32 semantics #29
Conversation
) | ||
|
||
(asserteq | ||
(invoke "eq_float32" (const.f32 1.123456789012345) (const.f32 1.123456789)) |
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.
For the AST syntax, it's desirable to issue an error when the literal value can't be exactly represented its type, rather than implicitly rounding it, as this test is testing, since rounding is an interesting operation from the perspective of a language spec.
There is also a variety of opinions about whether we even want a decimal syntax for floating point values, though I think it's fine for now for exactly representable values.
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.
I agree, it would be good to have the front-end issue errors whenever there is rounding. But it seems like that would require implementing our own string-to-float conversion. I'm no real expert on that.
At least I changed the tests so that the don't rely on rounding for literals.
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.
The proposed floating formats just represent the bits of the IEEE754
float32/float64 directly, so all you need at the end is an
Int64.float_of_bits
.
On Wed, Aug 26, 2015 at 1:01 PM, rossberg-chromium <notifications@github.com
wrote:
In ml-proto/test/float32.wasm
#29 (comment):
- (func $div_float32 (param $x f32) (param $y f32) (result f32)
- (div.f32 (getlocal $x) (getlocal $y))
- )
- (func $div_float64 (param $x f64) (param $y f64) (result f64)
- (div.f64 (getlocal $x) (getlocal $y))
- )
- (export "eq_float32" $eq_float32)
- (export "eq_float64" $eq_float64)
- (export "div_float32" $div_float32)
- (export "div_float64" $div_float64)
+)
+(asserteq
- (invoke "eq_float32" (const.f32 1.123456789012345) (const.f32 1.123456789))
I agree, it would be good to have the front-end issue errors whenever
there is rounding. But it seems like that would require implementing our
own string-to-float conversion. I'm no real expert on that.At least I changed the tests so that the don't rely on rounding for
literals.—
Reply to this email directly or view it on GitHub
https://github.com/WebAssembly/spec/pull/29/files#r38013736.
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.
A simple thing we could do for the parser for now would be to just test if string_of_float on the float value gives us a string equal to the string passed to float_of_string, and issue an error if it's not. That won't be everything we'll ever want, but it'll work for many simple purposes.
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.
Unfortunately, string representations for floats aren't unique. So that would prevent you from ever writing e.g. 1e10
, which would be rather bad in terms of usability.
But I agree that we should add hex float literals. I would still want to keep the ability to write human readable constants in the text format, though.
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.
Hex float is perfectly readable / writable by humans :-)
More seriously, I think it's pretty important that text parsers reject numbers which aren't precisely representable. It also important that all parsers know exactly how to do this so that they all accept the same inputs! Hex float is the easiest way to get this, but I'm open to supporting more.
I'd like to make sure that we discuss any other representation in WebAssembly/design#292, and that we write down the outcome so it's easy to search in the future. These PR discussions aren't searchable!
This will get us much closer but, unfortunately, there are some ops for which the
identity doesn't hold (e.g. modulus). The only ops that I know are proven are +, -, *, /, and sqrt. |
Fortunately, wasm doesn't have a floating point modulus operator :-). I think this technique covers all of the operators in wasm. |
Also, scanning for |
Ah, good point. Removed float mod. On 26 August 2015 at 16:12, Luke Wagner notifications@github.com wrote:
|
lgtm. I also felt the pain of the imprecise decimal representation in #24; perhaps we could open a new issue/PR to use one of the schemes from design/#292 as @sunfishcode suggested? |
@lukewagner's cross-repo link should have been WebAssembly/design#292 :-) I strongly support this proposal! |
Implement accurate float32 semantics
Use definitions from ECMAScript doc.
…able.copy`. (WebAssembly#29) This would make it simpler to extend those instructions to support multiple memories/tables, and copying between different memories/tables. The current encoding has a single placeholder zero byte for those instructions, which allows extension to multiple memories/tables, but would require a more complicated encoding to add two immediate indices.
* Add type annotations to continuation instructions This patch annotates the following continuation instructions with a continuation type: * `cont.bind $src $dst`: The annotation `$src` specifies the type of the source continuation, and `$dst` specifices the type of the target continuation (i.e. the continuation type after the partial continuation application). The latter type annotation was already present. * `resume $kt`: The annotation `$kt` specifices the type of the continuation argument to `resume`. * `resume $kt $tagt`: The annotation `$kt` specifies the type of the continuation argument to `resume_throw`, whilst `$tagt` specifices the type of the tag argument. The latter type annotation was already present. This patch also drops the verbose `(type $t)` syntax on continuation instructions in favour of simply `$t`. * Update interpreter/valid/valid.ml Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org> * Update interpreter/valid/valid.ml Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org> * Address Andreas' comments. * Update interpreter/valid/valid.ml Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org> * Update interpreter/valid/valid.ml Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org> * Address Andreas' feedback 2 --------- Co-authored-by: Andreas Rossberg <rossberg@mpi-sws.org>
Switch order of load/store immediates in binary format
Merge with wasm-3.0+exn
@lukewagner, @kg, doing it asm.js-style.