Open Bug 1511958 Opened Last year Updated 6 days ago

[meta] Implement i64<>JavaScript’s BigInt conversions proposal

Categories

(Core :: Javascript: WebAssembly, enhancement, P3)

enhancement

Tracking

()

ASSIGNED
Tracking Status
firefox70 --- fixed

People

(Reporter: sven, Assigned: asumu)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

(Keywords: leave-open, meta)

Attachments

(5 files, 1 obsolete file)

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) QtWebEngine/5.11.1 Chrome/65.0.3325.230 Safari/537.36

Steps to reproduce:

Take the following Wasm module: `(func (export "f")) (result i64) (i64.const 66))`.



Actual results:

Calling `f` is not possible in JavaScript because the signature contains an i64 type and there no way to represent it properly on the host currently.


Expected results:

JavaScript recently introduced the BigInt object, allowing the representation of arbitrary length integers.

The proposal is specifying the conversion between these two types.
Note that I can work on this, I'm currently implementing it in V8 as well.
Probably good to coordinate with the Igalia folks working on BigInt.
Blocks: js-bigint
Yes, I'm working for Igalia.

I forgot to mention that the proposal (https://github.com/WebAssembly/JS-BigInt-integration) is stage 2 in WebAssembly's GC process. I don't think it should block the JavaScript BigInt implementation since it should be considered as an experimental WebAssembly feature.
(In reply to Sven Sauleau from comment #3)
> Yes, I'm working for Igalia.

Oh, awesome \o/
Assignee: nobody → sven
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Depends on: 1512702
Depends on: 1512711
Attachment #9050016 - Attachment description: Implement i64<>JavaScript’s BigInt conversions proposal → Implement i64<>JavaScript’s BigInt conversions proposal (JS runtime)
Type: defect → enhancement

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:sven, could you have a look please?

Flags: needinfo?(sven)
Priority: -- → P3

Continues rev D22990 to implement the I64 to BigInt conversions proposal (https://github.com/WebAssembly/JS-BigInt-integration), original rev by Sven Sauleau

This is part 1 of a series of revs that split up D41710 into smaller revs. This rev just adds a compile-time flag for wasm BigInt to I64 conversion but does not use it anywhere, as its uses are for toggling features in subsequent revs.

This is part 2 of a series of revs that split up D41710 (for Wasm I64 to BigInt conversion) into smaller revs. This rev depends on the compile-time flag added in D43177 and adds a runtime flag to JSContext options that will toggle whether I64 to BigInt conversion is used. The flag will get used mostly in WasmInstance.cpp, but it also needs to be used to toggle I64 error checks in both Ion inlining code and in Wasm stub generation code. To pass that information along, the flag is also put in CompileArgs for WasmCompile and then copied to wasm module metadata (so that it can be read from lazy stub generation code).

Attachment #9050016 - Attachment is obsolete: true

Setting checkin-needed for https://phabricator.services.mozilla.com/D43177. @asumu I think it can make sense to open new bugs blocking this one when you have additional patches beyond the second one in the series that's already up.

Pushed by rgurzau@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d2072e34a3b1
Implement i64<>JavaScript’s BigInt conversions proposal (part 1, build system changes) r=wingo,bbouvier

Keywords: checkin-needed

https://phabricator.services.mozilla.com/D43179 needs review from bbouvier to continue. Thank you.

Assignee: sven → asumu

This is part 3 of a series of revs that split up D41710 (for Wasm I64 to BigInt conversion) into smaller revs. This rev depends on the compile-time flag added in D43177 and the run-time flag in D43179. Sorry for the delay in uploading this, there were a number of bugs I had to fix.

The rev adds support for BigInt<->I64 conversion in WasmInstance and to the Wasm stubs for the JIT, and in the inlining code in MCallOptimize. It addresses some of the main concerns that wingo brought up in comments on the original rev: the BigInt to I64 code path in the stubs is now written in assembly (in MacroAssembly.cpp) to avoid some hacky code. Most of the changes are also guarded by an ifdef for ENABLE_WASM_BIGINT now.

One comment that I haven't addressed yet but I'll add is BigInt fast paths for the JIT entry stub for simple cases.

This is part 4 of a series of revs that split up D41710 (for Wasm I64 to BigInt conversion) into smaller revs. This rev depends on the compile-time flag added in D43177 and the run-time flag in D43179 and the main implementation in D46205.

This rev adds tests in the JIT test suite that exercises the BigInt conversion code, and also modifies existing tests to work with the --wasm-big-int flag either enabled or disabled (with different expected results).

Is the spec rendered somewhere, please? In particular, I wanted to learn if/why it was fine to wrap an incoming BigInt into an i64 (since a big integer may be arbitrarily large, unless its name stands for a lie).

Flags: needinfo?(asumu)

bbouvier: it is now: https://webassembly.github.io/JS-BigInt-integration/js-api/index.html

BigInt values wrap around modulo 2^64, just like Numbers do for i32.

Flags: needinfo?(sven)
Flags: needinfo?(asumu)
Blocks: 1595031
You need to log in before you can comment on or make changes to this bug.