Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-128627: Emscripten: Use wasm-gc based call adaptor if available #128628

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

hoodmane
Copy link
Contributor

@hoodmane hoodmane commented Jan 8, 2025

Part of the ongoing quest to support JSPI. The JSPI spec removed its dependence on JS type reflection, and now the plan is for runtimes to ship JSPI while keeping type reflection in stage 3. So we need an alternative way to count the number of parameters of a function. It is possible to count them by repeatedly trying to instantiate a webassembly module with the function as an import of a different type signature. But this is pretty inefficient.

Since WebAssembly gc is now stage 4, there is a new option. WebAssembly gc added the ref.test instruction which can ask if a funcref has a given type. It's a bit difficult to apply because even our usual assembler the wasm binary toolkit doesn't support this instruction yet. But all JS engines that support JSPI support it. We just have to do some manual work to produce the binary.

This code also has to be written carefully to interact properly with memory snapshots. Importantly, no JS initialization code can be called from the C initialization code. For this reason, we make a C function pointer to fill from JS and fill it in a preRun function.

Part of the ongoing quest to support JSPI. The JSPI spec removed its dependence on
JS type reflection, and now the plan is for runtimes to ship JSPI while keeping
type reflection in stage 3. So we need an alternative way to count the number of
parameters of a function. It is possible to count them by repeatedly trying to
instantiate a webassembly module with the function as an import of a different type
signature. But this is pretty inefficient.

Since WebAssembly gc is now stage 4, there is a new option. WebAssembly gc added the
`ref.test` instruction which can ask if a funcref has a given type. It's a bit difficult
to apply because even our usual assembler the wasm binary toolkit doesn't support this
instruction yet. But all JS engines that support JSPI support it. We just have to do some
manual work to produce the binary.

This code also has to be written carefully to interact properly with memory snapshots.
Importantly, no JS initialization code can be called from the C initialization code.
For this reason, we make a C function pointer to fill from JS and fill it in a preRun
function.
Copy link
Contributor

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll admit I'm a little hazy on the specifics here; my biggest concern is your comment on the original ticket about toolchain support for wasm-gc being poor. Is that the reason for the "big blob of WASM"? i.e., that compilers can't (currently) generate the required WASM yet, so we have to hard-code the WASM to access the wasm-gc feature that is needed?

Assuming that's the case, I think this all makes sense - and at the very least, it's isolated to the Emscripten build (with the one exception in the pyruntimestate).

I've flagged a couple of other minor issues; plus this PR needs a NEWS entry.

Include/internal/pycore_runtime.h Outdated Show resolved Hide resolved
Python/emscripten_trampoline.c Show resolved Hide resolved
@bedevere-app
Copy link

bedevere-app bot commented Jan 9, 2025

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@hoodmane
Copy link
Contributor Author

hoodmane commented Jan 9, 2025

Is that the reason for the "big blob of WASM"? i.e., that compilers can't (currently) generate the required WASM yet, so we have to hard-code the WASM to access the wasm-gc feature that is needed?

Yeah exactly. Even the assembler I usually use can't produce this instruction. Ideally, there would be a webassembly clang intrinsic for checking if a function pointer has the right signature and then we could write this in C like:

if (__builtin_wasm_call_signature_matches((three_arg)func)) {
   return ((three_arg)func)(self, args, kw);
}
if (__builtin_wasm_call_signature_matches((two_arg)func)) {
   return ((two_arg)func)(self, args);
}
// ...
PyErr_SetString(PyExc_SystemError,
                "Handler has unexpected call signature");

and then __builtin_wasm_call_signature_matches((three_arg)func) would emit the

table.get $func_ptr_table $func
ref.test $three_arg_type

instructions.

Though to be honest, I don't really know how __builtin_wasm_call_signature_matches would work since it takes a type and a pointer as an argument. cc @pmatos who was at one point working on clang wasm-gc support.

@pmatos
Copy link

pmatos commented Jan 9, 2025

Is that the reason for the "big blob of WASM"? i.e., that compilers can't (currently) generate the required WASM yet, so we have to hard-code the WASM to access the wasm-gc feature that is needed?

Yeah exactly. Even the assembler I usually use can't produce this instruction. Ideally, there would be a webassembly clang intrinsic for checking if a function pointer has the right signature and then we could write this in C like:

if (__builtin_wasm_call_signature_matches((three_arg)func)) {
   return ((three_arg)func)(self, args, kw);
}
if (__builtin_wasm_call_signature_matches((two_arg)func)) {
   return ((two_arg)func)(self, args);
}
// ...
PyErr_SetString(PyExc_SystemError,
                "Handler has unexpected call signature");

and then __builtin_wasm_call_signature_matches((three_arg)func) would emit the

table.get $func_ptr_table $func
ref.test $three_arg_type

instructions.

Though to be honest, I don't really know how __builtin_wasm_call_signature_matches would work since it takes a type and a pointer as an argument. cc @pmatos who was at one point working on clang wasm-gc support.

So, at the moment there's no support for wasm gc in clang. It's correct what @hoodmane is saying. With wasmgc support in the toolchain the blob of wasm wouldn't be necessary anymore. However, while working on this the client that was paying us had a change of priorities and we couldn't finish this work. It's very unfortunate. If there's anyone wishing to fund the completion of this work, please let me know or contact Igalia directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants