Closed Bug 1595031 Opened 5 years ago Closed 4 years ago

Implement wasm multi-value js api


(Core :: JavaScript: WebAssembly, task, P2)






(Reporter: Ms2ger, Unassigned)




OS: Unspecified → All
Priority: -- → P3
Hardware: Unspecified → All

Notably, there is real hair here in the jitted stubs layer because we are not currently rooting objects along certain return paths because there is only one return value; if we need to box that value in an allocated object then normal protocols ensure that it is properly rooted. Once we have more than one value that can be boxed, the earlier ones must be rooted while the latter are being boxed. Bigint will have the same problem. I've cross-referenced a couple of bugs that are doing that work now.

Depends on: 1581572, 1511958

And I should add, there are mechanisms (landing with bug 1581572) for functions that return multiple values to opt out of the fast jit stubs at a fine level of granularity, so it is possible to get the multi-value returns working before handling the fast jit stubs.

Priority: P3 → P2

The problem with multi-value, reftypes, and the JS API (stubs) is that the stack result area has nonuniform shape, which complicates GC.

Let's say you have a function that returns (anyref i32 i32). The first two results will go to memory and the last will be in a register. Let us assume that we want to allow entry from the interpreter and from the JIT, but we will punt on fast JIT stubs to simplify things.

The problem to me is that I don't see a nice way for stubs to properly record the location of the anyref. If you reserve space on the stack and pass a pointer to the stack, you will need a stack map, and stubs currently don't do that.

If instead you heap-allocate a temporary custom object, you can put the ResultType into the object and have a custom visitor use the it to direct GC visits -- can be quite convenient, especially if we might need to allocate to wrap the component results (e.g. int64 to BigInt) -- but that object would itself need to be rooted during the call!

I am not sure what the solution might be.

Yeah, that's tricky. And the stubs are becoming increasingly complex even without this complexity.

Speaking for myself, you could just do the interpreter entry here in case that simplifies anything, and we could punt even normal JIT entry to later, as we did for anyref initially.

For the wasm->js calls (exits), I think that GC doesn't pose a specific problem, because the wasm caller is responsible for rooting the values. I think I can manage to implement interpreter exits with no issue.

For js->wasm calls (entries), I had a thought: perhaps we should generate webassembly stubs -- like, stubs written in WebAssembly. If you have:

  (function $f (export "f") (result i32 anyref f64)

Then you would generate the equivalent of:

  (import "env" "void-to-i32-anyref-f64" $target
    (func $target (result i32 anyref f64)))
  (import "env" "make-array"
    (func $make-array (param i32) (result anyref)))
  (import "env" "array-set-i32"
    (func $array-set-i32 (param i32 anyref i32)))
  (import "env" "array-set-f64"
    (func $array-set-f64 (param f64 anyref i32)))
  (import "env" "array-set-anyref"
    (func $array-set-anyref (param anyref anyref i32)))
  (function export "void-to-i32-anyref-f64-stub" (result anyref)
    (local $array anyref)
    (call $target)
    (i32.const 3)
    (call $make-array)
    (local.get $array)
    (i32.const 2)
    (call $array-set-f64)
    (local.get $array)
    (i32.const 1)
    (call $array-set-anyref)
    (local.tee $array)
    (i32.const 0)
    (call $array-set-i32)
    (local.get $array)))

and instantiate the stub module with the target function, as well as with the helpers. edit: updated order of results

Hrm, I think I might prefer to have a discussion about how we might introduce rooting in the entry stubs before I go down that path...

Depends on: 1622828
Depends on: 1625887
Blocks: 1625927

Done with only support for intercalling with interpreted JS code.

Closed: 4 years ago
Resolution: --- → FIXED
See Also: → 1627620
You need to log in before you can comment on or make changes to this bug.