1248598 - wasm: some i64 changes

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Jan de Mooij [:jandem]]

Attachment: Part 1 - Some code required for i64/const

Filing this bug for a number of i64 related changes.

This first patch adds some code required for i64/const. Note that I left the NYI validation errors in Wasm.cpp, so this won't let us handle anything new, but it's a start.

(Before I remove the validation errors, I want to add the code to reject i64 imports/exports, and anything else required to not blow up fuzzing.)

Comment 1

[Luke Wagner [:luke]]

Comment on attachment 8719778 [details] [diff] [review]
Part 1 - Some code required for i64/const

Review of attachment 8719778 [details] [diff] [review]:
-----------------------------------------------------------------

Nice!

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Part 2 - Reject i64 imports/exports

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Support i64 on x64

Support i64, for now only on x86_64.

Comment 4

[Jan de Mooij [:jandem]]

It's hard to write int64 tests now, because all we have is i64/const (and we can't import/export i64). After I fix some regalloc stuff, I want to add i32.wrap/i64 and i64.eq et al, and then we can start writing tests.

It'd be great if we could import the .wast tests soon though, so I don't have to workaround the import/export problem :)

Comment 5

[Jan de Mooij [:jandem]]

Comment on attachment 8720716 [details] [diff] [review]
Part 3 - Support i64 on x64

Hm I didn't try it but this will probably assert when passing i64.const to a function. Let's hold off for now.

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b34a59d6ffe8

Comment 7

[Luke Wagner [:luke]]

Comment on attachment 8720707 [details] [diff] [review]
Part 2 - Reject i64 imports/exports

Review of attachment 8720707 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit-test/tests/wasm/basic.js
@@ +336,5 @@
>      assertThrowsInstanceOf(() => i2i(bad, 0), RangeError);
>      assertThrowsInstanceOf(() => i2v(bad, 0), RangeError);
>  }
> +
> +// When the test below starts failing, remove it and uncomment the lines below!

haha, good one

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Part 3 - Support i64 on x64

This patch enables i64, only on x64 for now.

Also some LIR changes to handle int64 registers, similar to what we do for boxed Values, and codegen for Int64 constants.

Most of these backend changes will work on all platforms, except for some argument and return value stuff. I'll look at that later, after we have the basic ops and can write useful tests.

Comment 9

[Dan Gohman [:sunfish]]

Comment on attachment 8720885 [details] [diff] [review]
Part 3 - Support i64 on x64

Review of attachment 8720885 [details] [diff] [review]:
-----------------------------------------------------------------

Cool!

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b34a59d6ffe8

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/eae4090bb9c7
https://hg.mozilla.org/mozilla-central/rev/7bfe45ec123a