add component-model-async/{fused|futures|streams}.wast tests #10106
Conversation
dicej
force-pushed
the
async-fused-test
branch
3 times, most recently
from
7e41099 to
098bd1c
Compare
github-actions
bot
added
the
wasmtime:api
There was a problem hiding this comment.
Ok I'm trying to decipher trampoline.rs but if it's ok with you I'd prefer to ask for more documentation first before diving further into these methods. Currently there's very little documentation on how things are set up and understanding enough to be able to review this PR I think would require cross-referencing both all the details in the spec along with Wasmtime internal implementation details. A lot of this is also pretty specific to Wasmtime itself in the sense that it's all internal adapter details and we're implementing halves of the spec in some places.
Would you be ok writing up some docs for this? Ideally I'd find it most helpful to have a high-level description of how the adapters piece together and what the expected flow between them is. My hope is that such documentation would also be pretty valuable for future readers to understand this as well.
There was a problem hiding this comment.
I'm sorry I know I sound like a broken record but I'm personally still very lost trying to understand this. On one hand one way I can fix this is to go study the component-model specification, try to piece together what an expected implementation would be, and then try to connect those dots back to this implementation. On the other hand though I also think it'd be valuable to have enough local documentation to not require that because although I can do that it would also be required for any future readers as well.
If you feel like I'm requesting too much documentation though or there's something that is well outside the purview of this module's documentation and it's more basic fundamental understanding please let me know though. I don't think we should just mirror the entire specification in comments into this repository, but at the same time I still think there are critical implementation pieces lacking comments such as the protocol between the async-start/return adapters as well as the host-provided async-enter/exit functions.
|
To be clear: I absolutely agree that more docs and comments are needed -- I've only added a bit of that so far but plan to add more. Feel free to ignore this PR until that's done. |
This is another piece of bytecodealliance#9582 which I'm splitting out to make review easier. The fused.wast test exercises fused adapter generation for various flavors of intercomponent async->async, async->sync, and sync->async calls. The futures.wast and streams.wast tests exercise the various intrinsics (e.g. `stream.read`, `future.close_writable`, etc.) involving `future`s and `stream`s. The remaining changes fill in some TODOs to make the tests pass, plus plumbing for a few intrinsics which aren't needed for these tests but which set the foundation for future tests. Signed-off-by: Joel Dice <[email protected]>
dicej
changed the title
|
@alexcrichton I've rebooted this PR based on the latest code in the This PR now includes two more tests ( |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "fuzzing", "wasmtime:api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
| @@ -114,6 +114,7 @@ pub type VMLoweringCallee = extern "C" fn( | |||
| vmctx: NonNull<VMOpaqueContext>, | |||
| data: NonNull<u8>, | |||
| ty: u32, | |||
| caller_instance: u32, | |||
There was a problem hiding this comment.
Mind documenting this parameter above? (perhaps documenting that it's currently-unused but soon-to-be-used as well)
|
Thanks for the review, @alexcrichton! I'll address your feedback first thing tomorrow. |
Signed-off-by: Joel Dice <[email protected]>
| /// Import a host built-in function to start a subtask for a sync-lowered | ||
| /// import call to an async-lifted export. | ||
| /// | ||
| /// This call with block until the subtask has produced result(s) via the |
| } | ||
|
|
||
| // This closure compiles a function to be exported to the host which host to | ||
| // lift the parameters from the caller and lower them to the callee. |
There was a problem hiding this comment.
to be exported to the host which host to lift the parameters ... maybe correct but I didn't know how to parse that part of the sentence.
| i32::try_from(self.types[adapter.lift.ty].results.as_u32()).unwrap(), | ||
| )); | ||
| // Async-lowered imports pass params and receive results via linear | ||
| // memory, and those pointers are in the the first and second params to |
| self.finish() | ||
| } | ||
|
|
||
| /// Compiles a function to be exported to the host which host to lift the |
There was a problem hiding this comment.
exported to the host which host to lift ... apologies if this makes sense to folks more in the know, maybe just hard for me to decipher
* Do proper subtype checking for imported globals during instantiation (bytecodealliance#10304) * pulley: Fix a panic compiling with debug info (bytecodealliance#10305) Debug into doesn't work on Pulley anyway but it's better to return a first-class error rather than a panic. This commit fills out a simple missing instruction in the Pulley backend to ensure that compilation gets far enough to the DWARF transform where there's no pulley support and an error is returned. * Improve rebuild detection of test-programs (bytecodealliance#10303) This commit improves the logic of detecting when to rebuild the `test-programs` artifacts used during test by parsing the `*.d` files that Cargo emits as part of its compilation and using that as the `cargo:rerun-if-changed` directive. This not only includes what was previously depended on but additionally includes features such as `path` dependencies which might temporarily be used during development. * Add a missing `apt-get update` before install (bytecodealliance#10310) I always forget this and it always bites us within a few months. Alas. * wasmtime-wit-bindgen: gen is a reserved keyword in the lexer starting in 2024 edition (bytecodealliance#10308) * Test natively on AArch64, not emulated (bytecodealliance#10306) This commit updates the aarch64 tests to use the new `*-arm` images from GitHub instead of running through QEMU emulation. This should be a bit faster but primarily helps build confidence that it runs on real hardware. I'll note that release builds aren't updated at this time to run on native hardware since that's moreso about glibc compatibility and it's a bit easier to keep the setup we currently have for that. If `*-arm` machines are noticably faster than the default x64 machines then we could in theory move everything over to an aarch64-based machine as opposed to just the aarch64 build. * chore: fix some typos in comments (bytecodealliance#10309) Signed-off-by: shenpengfeng <[email protected]> * Rename `VMRuntimeLimits` to `VMStoreContext` (bytecodealliance#10307) Way back in time, this struct originally contained the stack and fuel limits. Then it also got the epoch deadline. Then it also got the exit FP/PC and entry FP. Now it is just the place where we put per-store mutable data that is accessed by JIT code and must be shared between all `VMContext`s. So it is time to rename it. This commit is purely mechanical and just renames the type and various methods and variables that use/access it. * wasmtime-wit-bindgen: emit a definition for all types in a wit interface (bytecodealliance#10311) * wasmtime-wit-bindgen: emit a definition for all types in a wit The calculation of TypeInfo only reaches types which are passed to or from a function. For types which are not reachable, default to the defining them according to the ownership setting given to bindgen. I have my doubts that `with`-reuse of bindgen types actually works properly when bindgen is set to Ownership::Borrowing but thats out of scope for this PR, which is to fix bytecodealliance#10090 * component-macro: bless bindgen test output * asm: sse orpd implementation (bytecodealliance#10273) * sse orpd implementation assembler integration with isle format add clippy reason, reorder avx priority in isle bless tests for orpd create separate xmm module validate function rewrite sse condition add quote from manual for sse prefix format changes move Xmm bits under Reg * use new isle constructors for sse * remove unused function * minor changes * Winch: Fix consts and multivalue returns (bytecodealliance#10315) * fixed broken link (bytecodealliance#10318) * chore: fix parenthesis balance (bytecodealliance#10317) parentheses are not balanced here * winch(aarch64): ABI integration (bytecodealliance#10312) * winch(aarch64): ABI integration This commit finalizes the ABI integration between Winch and Cranelift, notably: * Updates the Cranelift ABI to ensure that all the Winch register clobbers are taken into account. * Updates the Winch ABI to treat x28 as callee-saved, since it's used as the shadow stack pointer. The alternative to treating x28 as callee-saved is to treat it as caller-saved and save it when required e.g., at call-sites, even though this approach works, it's probably more efficient to perform a store/pop once per function, to minimize the number of move instructions required. There are still some changes needed in order to fully enable running spec tests for aarch64, however, this change is one step further. If interested, you can run the call.wast test via: cargo run -- wast -Ccompiler=winch tests/spec_testsuite/call_indirect.wast * Update disas tests * Use `alloc_zeroed` to allocate dynamic table elements (bytecodealliance#10313) * Use `alloc_zeroed` to allocate dynamic table elements This allows us to get pre-zeroed memory from the global allocator, rather than needing to manually zero-initialize the elements. * Don't ask the global allocator to allocate a block of size zero * x64: use Rust types for assembler immediates (bytecodealliance#10302) Previously we used `AssemblerImm*` and `AssemblerSimm*` throughout the x64 ISLE to indicate the type of immediate an instruction would receive. Alex has noted previously that this is unnecessary; it does after all create more ties between the `cranelift-codegen` ISLE and `cranelift-assembler-x64` that could possibly break in the future. This change removes those ties by using Rust types in ISLE (e.g., `u8`, `i8`, `u16`, etc.) and converting to the expected assembler type in the ISLE glue layer. * Update ir.md (bytecodealliance#10319) Hello, A possible typo in this text: "Forward" Should be "Foreword" "Forward" means to move ahead, but the correct term for an introduction is "Foreword". Thanks. * Expose GC refs to Wasm in `gc_alloc_raw` libcall (bytecodealliance#10322) * Expose GC refs to Wasm in `gc_alloc_raw` libcall As we are returning a GC reference to Wasm, we need to mark that GC reference as exposed to Wasm. Fixes bytecodealliance#9669 * miri ignore test that calls wasm and therefore can't run in miri * Bump Wasmtime to 32.0.0 (bytecodealliance#10330) Co-authored-by: Wasmtime Publish <[email protected]> * add component-model-async/{fused|futures|streams}.wast tests (bytecodealliance#10106) * add component-model-async/{fused|futures|streams}.wast tests This is another piece of bytecodealliance#9582 which I'm splitting out to make review easier. The fused.wast test exercises fused adapter generation for various flavors of intercomponent async->async, async->sync, and sync->async calls. The futures.wast and streams.wast tests exercise the various intrinsics (e.g. `stream.read`, `future.close_writable`, etc.) involving `future`s and `stream`s. The remaining changes fill in some TODOs to make the tests pass, plus plumbing for a few intrinsics which aren't needed for these tests but which set the foundation for future tests. Signed-off-by: Joel Dice <[email protected]> * address review feedback Signed-off-by: Joel Dice <[email protected]> --------- Signed-off-by: Joel Dice <[email protected]> * Update wasm-tools to the latest revision (bytecodealliance#10314) * Update wasm-tools to the latest revision * Update component async bits for new intrinsics * Ignore options for now * I truly, and fundamentally, do not understand `cargo vet` * Fix a test * Ignore fuzzing `testcase0.wasm` et al in `.gitignore` (bytecodealliance#10341) * remove temporary patch from Cargo.toml Signed-off-by: Joel Dice <[email protected]> * Fix building artifacts alone in isolation Few more crate features needed * Rename backpressure intrinsic * Patch to work-in-progress branches * temporarily stub-out waitable-set.wait calls in tests Signed-off-by: Joel Dice <[email protected]> * fix WAT/WAST test regressions Signed-off-by: Joel Dice <[email protected]> * Switch to published wasm-tools deps * bless bindgen test output Signed-off-by: Joel Dice <[email protected]> * remove realloc from `task.return` in WAT tests This is no longer allowed by wasmparser. Signed-off-by: Joel Dice <[email protected]> * Switch to wit-bindgen upstream --------- Signed-off-by: shenpengfeng <[email protected]> Signed-off-by: Joel Dice <[email protected]> Co-authored-by: Nick Fitzgerald <[email protected]> Co-authored-by: Alex Crichton <[email protected]> Co-authored-by: Pat Hickey <[email protected]> Co-authored-by: shenpengfeng <[email protected]> Co-authored-by: Rahul <[email protected]> Co-authored-by: Jeffrey Charles <[email protected]> Co-authored-by: ifsheldon <[email protected]> Co-authored-by: Bongjun Jang <[email protected]> Co-authored-by: Saúl Cabrera <[email protected]> Co-authored-by: Andrew Brown <[email protected]> Co-authored-by: owen <[email protected]> Co-authored-by: wasmtime-publish <[email protected]> Co-authored-by: Wasmtime Publish <[email protected]>
This is another piece of #9582 which I'm splitting out to make review easier.
The fused.wast test exercises fused adapter generation for various flavors of
intercomponent async->async, async->sync, and sync->async calls.
The futures.wast and streams.wast tests exercise the various intrinsics
(e.g.
stream.read,future.close_writable, etc.) involvingfutures andstreams.The remaining changes fill in some TODOs to make the tests pass, plus plumbing
for a few intrinsics which aren't needed for these tests but which set the
foundation for future tests.