Update stack switching API to match V8

[aheejin]

Per
v8/v8@ff44012,
this moves returnPromiseOnSuspend and suspendOnReturnedPromise from
Suspender to WebAssembly, and removes the uses of
Asyncify.suspender from the code.

@fgmccabe @thibaudmichaud

[aheejin]

The CI errors out with:

AssertionError: JS subprocess failed (/root/.jsvu/v8 --wasm-staging --wasm-staging --experimental-wasm-stack-switching /tmp/tmp05ur8_u3/emscripten_test_core0_wc2dn5gh/main.js --no-wasm-async-compilation --): 1.  Output:
failed to asynchronously prepare wasm: LinkError: WebAssembly.instantiate(): Import #1 module="env" function="emscripten_sleep" error: imported function does not match the expected type
Aborted(LinkError: WebAssembly.instantiate(): Import #1 module="env" function="emscripten_sleep" error: imported function does not match the expected type)
/tmp/tmp05ur8_u3/emscripten_test_core0_wc2dn5gh/main.js:813: RuntimeError: Aborted(LinkError: WebAssembly.instantiate(): Import #1 module="env" function="emscripten_sleep" error: imported function does not match the expected type)
  var e = new WebAssembly.RuntimeError(what);
          ^
RuntimeError: Aborted(LinkError: WebAssembly.instantiate(): Import #1 module="env" function="emscripten_sleep" error: imported function does not match the expected type)
    at abort (/tmp/tmp05ur8_u3/emscripten_test_core0_wc2dn5gh/main.js:813:11)
    at /tmp/tmp05ur8_u3/emscripten_test_core0_wc2dn5gh/main.js:992:7

1 pending unhandled Promise rejection(s) detected.

Log link: https://app.circleci.com/pipelines/github/emscripten-core/emscripten/22753/workflows/63fcd61c-b8e2-4125-8c91-59fbdf05c871/jobs/555771

@fgmccabe @thibaudmichaud Do you have any idea why it complains about the incorrect signature of emscripten_sleep? Did anything in V8 change recently to cause this?

[thibaudmichaud]

Thanks for working on this!

Yes, I know why it complains. The PR seems to be missing a crucial difference of the static API: the export and import functions now expect an additional Suspender parameter as an externref to explicitly connect an import to an export. This means that Emscripten needs to generate additional wasm code to manage the Suspender object(s).

Here is a before/after comparison with a small example:

Before:

let suspender = new WebAssembly.Suspender();
let wrapped_import = suspender.suspendOnReturnedPromise(
  new WebAssembly.Function({params: [], results: ['i32']}, js_import);
let instance = WebAssembly.instantiate(..., {m: {import: wrapped_import}});
let wrapped_export = suspender.returnPromiseOnSuspend(instance.exports.export);
let promise = wrapped_export();

(module
  (import $import "m" "import" (func (result i32)))
  (func (export "export") (result i32)
    (call $import) ;; Suspend when $import returns a promise
  )
)

After:

let wrapped_import = WebAssembly.suspendOnReturnedPromise(
  new WebAssembly.Function({params: [], results: ['i32']}, js_import);
let instance = WebAssembly.instantiate(..., {m: {import: wrapped_import}});
let wrapped_export = WebAssembly.returnPromiseOnSuspend(instance.exports.export);
let promise = wrapped_export();

(module
  (import $import "m" "import" (func (param externref) (result i32)))
  (func (export "export") (param $suspender externref) (result i32)
    (call $import (local.get $suspender)) ;; Suspend $suspender when $import returns a promise
  )
)

Here I pass the suspender using the local index directly, but I imagine that Emscripten will want to store it in a global for instance, so that it can be loaded from anywhere in the module.
Note that on the JavaScript side the Suspender parameter doesn't exist, it gets created by the export wrapper, and is consumed by the import wrapper, so it is only visible to wasm.
The point of the static API is to allow reentrancy: each call to the wrapped export uses a different Suspender. This entails some significant changes to Emscripten to manage the multiple Suspenders. But supporting the previous single-Suspender scenario as a first step would already be good.

The API was implemented in a series of commit in V8, and the one you linked is not the last one. This is the first commit where the implementation is complete:
https://chromium-review.googlesource.com/c/v8/v8/+/3780817

Also FYI, there is another smaller API change in flight:
WebAssembly/js-promise-integration#8
So you might want to wait until then to update Emscripten. I am currently working on it, I'll let you know when it lands.

After this change, my example above would look like this:

let wrapped_import = new WebAssembly.Function({params: [], results: ['externref']}, js_import, {suspending: 'first'});
let instance = WebAssembly.instantiate(..., {m: {import: wrapped_import}});
let wrapped_export = new WebAssembly.Function({params: [], results: ['externref']}, instance.exports.export, {promising: 'first'});
let promise = wrapped_export();

[aheejin]

Thanks! I'm not sure on how the signature changed though.. (Your 'before' and 'after' code's signatures look the same despite that the latter is in WebAssembly class..?)

So now every function needs externref arg in parameters or results or both? Don't we already have externref in the type alerady?

emscripten/src/library_async.js

Lines 104 to 107 in 0e75ea8

	// Regardless of the original result type of the function, as it
	// is now expected to potentially return a Promise, change it to
	// an externref.
	type.results = ['externref'];

[thibaudmichaud]

Thanks! I'm not sure on how the signature changed though.. (Your 'before' and 'after' code's signatures look the same despite that the latter is in WebAssembly class..?)

In the JS code, yes, they are the same. In the wasm module the signatures are different, they take an additional externref parameter. This parameter is not visible to JS because it gets created by the JS-to-wasm wrapper, and gets discarded by the wasm-to-JS wrapper.

The code you pasted is for the result type. It's an externref so we can return a Promise, that doesn't change. This is unrelated to the Suspender externref argument.

BTW the upcoming API change that I mentioned at the end of my previous comment is now in V8:
https://chromium-review.googlesource.com/c/v8/v8/+/3804669

There have been some subtle changes to the typing rules, so here is the full example again for reference:

function js_import() { return Promise.resolve(0); }
let wrapped_import = new WebAssembly.Function({params: ['externref'], results: ['i32']}, js_import, {suspending: 'first'});
let instance = WebAssembly.instantiate(..., {m: {import: wrapped_import}});
let wrapped_export = new WebAssembly.Function({params: [], results: ['externref']}, instance.exports.export, {promising: 'first'});
let promise = wrapped_export();

(module
  (import $import "m" "import" (func (param externref) (result i32)))
  (func (export "export") (param $suspender externref) (result i32)
    (call $import (local.get $suspender)) ;; Suspend $suspender when $import returns a promise
  )
)

Please see the PR for a more detailed explanation and for the general typing rules.

[aheejin]

Thanks @thibaudmichaud for the detailed explanation! Apparently this is more complicated than I expected... We need not only JS API changes but the generated wasm code should change. I'll put this on hold then for the moment.

[aheejin]

I think it has been taken care a while ago by @brendandahl.