Closed Bug 1248598 Opened 8 years ago Closed 8 years ago

wasm: some i64 changes


(Core :: JavaScript Engine: JIT, defect)

Not set



Tracking Status
firefox47 --- fixed


(Reporter: jandem, Assigned: jandem)




(3 files, 1 obsolete file)

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 on attachment 8719778 [details] [diff] [review]
Part 1 - Some code required for i64/const

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

Attachment #8719778 - Flags: review?(luke) → review+
Attachment #8720707 - Flags: review?(luke)
Attached patch Part 3 - Support i64 on x64 (obsolete) — Splinter Review
Support i64, for now only on x86_64.
Attachment #8720716 - Flags: review?(luke)
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 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)
Keywords: leave-open
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+
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 on attachment 8720885 [details] [diff] [review]
Part 3 - Support i64 on x64

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

Attachment #8720885 - Flags: review?(sunfish) → review+
Keywords: leave-open
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.