Extend number of flat parameters in async lower from 1 to 4 #520
+106
−51
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alexcrichton
approved these changes
May 21, 2025
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
May 21, 2025
This commit updates wasm-tools with the changes from WebAssembly/component-model#520 which updates the ABI of async imports and exports. The changes here are: * Flat function parameters are now allowed. * If there are no function parameters then the parameters are entirely omitted. * The result out-ptr is now omitted if there are no function results.
| @@ -2466,6 +2466,8 @@ def flatten_functype(opts, ft, context): | |||
| else: | |||
| match context: | |||
| case 'lift': | |||
| if len(flat_params) > MAX_FLAT_PARAMS: | |||
There was a problem hiding this comment.
To confirm, should this be MAX_FLAT_ASYNC_PARAMS?
There was a problem hiding this comment.
Since we're talking about the incoming direction, the motivation for limiting outgoing to 4 doesn't apply, so I hadn't thought to change it, but does it simplify adapter fusion in cases where the ABI options cause the core lifted/lowered signatures to line up?
There was a problem hiding this comment.
It looks like wasip3-prototyping already supports the limit here as 16, and already has adapters for memory->flat, so I think it's reasonable to leave this as-is (and I agree the motivation for 4 is not relevant here).
I'll assume this'll stick to 16 and see how far I get
alexcrichton
added a commit
to alexcrichton/wasm-tools
that referenced
this pull request
May 21, 2025
This commit updates wasm-tools with the changes from WebAssembly/component-model#520 which updates the ABI of async imports and exports. The changes here are: * Flat function parameters are now allowed. * If there are no function parameters then the parameters are entirely omitted. * The result out-ptr is now omitted if there are no function results.
alexcrichton
added a commit
to alexcrichton/wasip3-prototyping
that referenced
this pull request
May 22, 2025
This commit updates to account for WebAssembly/component-model#520. This is a large-ish change to the semantics from a runtime perspective and needed a number of changes: * The `sync_prepare_call` and `async_prepare_call` libcalls were merged together. The previous async version statically took two arguments but now it's taking a variable number of arguments which looked quite a lot like `sync_prepare_call` so they're now merged into one. * Lots of little updates were made to `fact/signatures.rs` to account for ABI changes. * Tests with handwritten signatures were all updated to the new ABI. * The `CallerInfo::Async` structure which "buffers" a call was updated to have a `Vec<ValRaw>` for the incoming parameters. This is a particularly inefficient way to store parameters but it's in theory workable for now. * The `Storage` abstraction in host calls was refactored and updated to account for async and how lifting parameters could be either flat or indirect. Similar updates were made to the dynamic path as well
|
Ok this ended up being simpler than I predicted (yay!) to implement. I've updated:
I'm going to wait to merge things until Joel is back next week, but otherwise this looks good to me |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As suggested in #434, this PR increases the number of flattened parameters allow in async-lowered calls from 1 to 4. (This is different than the
MAX_FLAT_ARGSof 16 for sync-lowered calls because runtimes will likely need to reserve space for the flat argument tuple in an inline fixed-size "subtask" structure.)For consistency, it would also make sense to change
future.writeto take flattened arguments, but this is a larger change (at least in the spec text) since futures are defined as specialized streams which work in terms of buffers, so I was thinking of breaking that out into a separate PR.