Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: jandem, Assigned: jandem)

Tracking

unspecified
mozilla47
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Assignee

Description

3 years ago
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]:
-----------------------------------------------------------------

Nice!
Attachment #8719778 - Flags: review?(luke) → review+
Assignee

Comment 2

3 years ago
Attachment #8720707 - Flags: review?(luke)
Assignee

Comment 3

3 years ago
Posted patch Part 3 - Support i64 on x64 (obsolete) — Splinter Review
Support i64, for now only on x86_64.
Attachment #8720716 - Flags: review?(luke)
Assignee

Comment 4

3 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

3 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

3 years ago
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+
Assignee

Comment 8

3 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 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+
Assignee

Updated

3 years ago
Keywords: leave-open

Comment 11

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/eae4090bb9c7
https://hg.mozilla.org/mozilla-central/rev/7bfe45ec123a
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.