Closed
Bug 1248598
Opened 9 years ago
Closed 9 years ago
wasm: some i64 changes
Categories
(Core :: JavaScript Engine: JIT, defect)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla47
Tracking | Status | |
---|---|---|
firefox47 | --- | fixed |
People
(Reporter: jandem, Assigned: jandem)
References
Details
Attachments
(3 files, 1 obsolete file)
6.54 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
3.34 KB,
patch
|
luke
:
review+
|
Details | Diff | Splinter Review |
31.98 KB,
patch
|
sunfish
:
review+
|
Details | Diff | Splinter Review |
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.)
Attachment #8719778 -
Flags: review?(luke)
![]() |
||
Comment 1•9 years ago
|
||
Comment on attachment 8719778 [details] [diff] [review]
Part 1 - Some code required for i64/const
Review of attachment 8719778 [details] [diff] [review]:
-----------------------------------------------------------------
Nice!
Attachment #8719778 -
Flags: review?(luke) → review+
Assignee | ||
Comment 2•9 years ago
|
||
Attachment #8720707 -
Flags: review?(luke)
Assignee | ||
Comment 3•9 years ago
|
||
Support i64, for now only on x86_64.
Attachment #8720716 -
Flags: review?(luke)
Assignee | ||
Comment 4•9 years ago
|
||
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 :)
Assignee | ||
Comment 5•9 years ago
|
||
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.
Attachment #8720716 -
Flags: review?(luke)
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
![]() |
||
Comment 7•9 years ago
|
||
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
Attachment #8720707 -
Flags: review?(luke) → review+
Assignee | ||
Comment 8•9 years ago
|
||
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.
Attachment #8720716 -
Attachment is obsolete: true
Attachment #8720885 -
Flags: review?(sunfish)
Comment 9•9 years ago
|
||
Comment on attachment 8720885 [details] [diff] [review]
Part 3 - Support i64 on x64
Review of attachment 8720885 [details] [diff] [review]:
-----------------------------------------------------------------
Cool!
Attachment #8720885 -
Flags: review?(sunfish) → review+
Comment 10•9 years ago
|
||
bugherder |
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 11•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/eae4090bb9c7
https://hg.mozilla.org/mozilla-central/rev/7bfe45ec123a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox47:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in
before you can comment on or make changes to this bug.
Description
•