Plumb initial translation of thread.spawn_indirect
#10518
Conversation
| Trampoline::ThreadSpawnIndirect { ty: _, table } => { | ||
| // TODO: eventually pass through the `ty` argument to check the | ||
| // table's funcref signature. | ||
| self.translate_thread_spawn_indirect(*table) |
There was a problem hiding this comment.
@alexcrichton, I feel like there is some way to actually materialize this ty index into a real type so we can check that the funcref has the expected type of the thread start function but I don't see it.
There was a problem hiding this comment.
E.g., is TypeFuncIndex the right type to be passing around here?
There was a problem hiding this comment.
TypeFuncIndex is fine yeah, and the way to pass it to runtime would basically be to embed the constant in the generated code itself. Runtime would then be responsible for translating TypeFuncIndex into a VMSharedTypeIndex which can be done through various translation tables similar to how core wasm does it
There was a problem hiding this comment.
Ok, but actually the index being used is coming from a core function type. E.g., in the tests, for (core type $start (shared (func (param $context i32)))), we use $start as the index passed through to this canonical built-in. I see how to retrieve a TypeFunc using ComponentTypes but I don't think that's what we actually want here — that would be a component type. Is TypeFuncIndex the right index to be using here? How do we get to its core type (presumably backed by VMSharedTypeIndex)?
There was a problem hiding this comment.
Agreed yeah we don't want to deal with TypeFunc here as the type-check should be a single comparison ideally, via VMShareTypeIndex. Converting from TypeFuncIndex will require a table of some form stored in VMComponentContext which is filled in on initialization. That happens for core modules, for example, where basically VMContext has a table from TypeFuncIndex to VMSharedTypeIndex
github-actions
bot
added
the
wasmtime:api
There was a problem hiding this comment.
This seems reasonable enough to me with some tweaks, but I do think we'll want a test for this. I don't think there's any viable path to testing the runtime bits at this time so I'd prefer to leave the implementation body of the intrinsic as todo!() for now, but testing at least the compile-time/Cranelift parts seems reasonable with a test that just compiles something but does nothing with the result.
| Trampoline::ThreadSpawnIndirect { ty: _, table } => { | ||
| // TODO: eventually pass through the `ty` argument to check the | ||
| // table's funcref signature. | ||
| self.translate_thread_spawn_indirect(*table) |
There was a problem hiding this comment.
TypeFuncIndex is fine yeah, and the way to pass it to runtime would basically be to embed the constant in the generated code itself. Runtime would then be responsible for translating TypeFuncIndex into a VMSharedTypeIndex which can be done through various translation tables similar to how core wasm does it
| let table = unsafe { | ||
| Instance::from_vmctx(vmctx.as_non_null(), |handle| { | ||
| let idx = handle.table_index(from.as_non_null().as_ref()); | ||
| handle | ||
| .get_defined_table_with_lazy_init(idx, iter::once(element)) | ||
| .as_ref() | ||
| }) | ||
| }; |
There was a problem hiding this comment.
FWIW technically some and/or most of this is going to go away. Lazy initialization is probably not going to be possible with shared tables.
There was a problem hiding this comment.
Yup. What I'm trying to do is plumb through enough to (a) show how this is currently possible using unshared tables (while still immediately failing) and (b) create a use site here so that future refactoring considers this usage. I was thinking that tests could even check for the initial behavior that is functional today: i.e., we can't run a shared function, but at least we can check that using the wrong index results in a failure.
There was a problem hiding this comment.
I don't think it's possible to hit this at runtime though? All components would fail validation unless they use shared tables, and shared talbes aren't implemented in Wasmtime, so it's not possible to instantiate any component using this intrinsic?
This change in no way implements actual spawning of threads; it simply translates the CM builtin `thread.spawn_indirect` to invoke a real Wasmtime libcall that does some preliminary checks and panics. It is useful, though, to (1) exercise all the new table-related functions and (2) show the limits of Wasmtime's current state: we will need a way of interacting with shared functions and possibly some form of shared store.
…etrieved spawn function
This test cannot pass until more work is done to translate `shared` types in Wasmtime.
abrown
force-pushed
the
set-indirect-libcall
branch
from
40e6322 to
434b258
Compare
This change in no way implements actual spawning of threads; it simply translates the CM builtin
thread.spawn_indirectto invoke a real Wasmtime libcall that does some preliminary checks and panics. It is useful, though, to (1) exercise all the new table-related functions and (2) show the limits of Wasmtime's current state: we will need a way of interacting with shared functions and possibly some form of shared store.