-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Initial skeleton of some component model processing #4005
Initial skeleton of some component model processing #4005
Conversation
It's worth pointing out that this is starting out as a draft PR. The main reason for this is that there aren't actually any tests in this PR for that. For tests I'm think we'll need bytecodealliance/wasm-tools#529 which is the text format for components. Tests can probably be written in the meantime with |
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "wasm-proposal: component-model", "wasmtime:api", "wasmtime:config"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
To modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo tests! I'll take another look at this once those are written
.translate(parser.clone(), &component[range.start..range.end])?; | ||
let upvar_idx = self.result.upvars.push(translation); | ||
self.result.modules.push(ModuleDef::Upvar(upvar_idx)); | ||
return Ok(Action::Skip(range.end - range.start)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if we added a len
method to wasmparser::Range
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider me sufficiently inspired
0f52568
to
677e35a
Compare
This commit removes `wasmparser::Range` and instead uses `std::ops::Range<usize>` instead. This is inspired by review on bytecodealliance/wasmtime#4005 where it's more convenience to have all the libstd methods available.
677e35a
to
96962a6
Compare
96962a6
to
5e73ec2
Compare
Ok with bytecodealliance/wasm-tools#529 landed I've now started writing tests for this PR. Once all the pending PRs on wasm-tools are landed I'll make a release and then this should be ready-for-landing after another round of review. |
Subscribe to Label Actioncc @fitzgen
This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "fuzzing"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
Subscribe to Label Actioncc @peterhuene
This issue or pull request has been labeled: "cranelift:module", "wasmtime:c-api"
Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
This commit updates these crates as used by Wasmtime for the recently published versions to pull in changes necessary to support the component model. I've split this out from bytecodealliance#4005 to make it clear what's impacted here and bytecodealliance#4005 can simply rebase on top of this to pick up the necessary changes.
* Update the wasm-tools family of crates This commit updates these crates as used by Wasmtime for the recently published versions to pull in changes necessary to support the component model. I've split this out from #4005 to make it clear what's impacted here and #4005 can simply rebase on top of this to pick up the necessary changes. * More test fixes
This commit is the first of what will likely be many to implement the component model proposal in Wasmtime. This will be structured as a series of incremental commits, most of which haven't been written yet. My hope is to make this incremental and over time to make this easier to review and easier to test each step in isolation. Here much of the skeleton of how components are going to work in Wasmtime is sketched out. This is not a complete implementation of the component model so it's not all that useful yet, but some things you can do are: * Process the type section into a representation amenable for working with in Wasmtime. * Process the module section and register core wasm modules. * Process the instance section for core wasm modules. * Process core wasm module imports. * Process core wasm instance aliasing. * Ability to compile a component with core wasm embedded. * Ability to instantiate a component with no imports. * Ability to get functions from this component. This is already starting to diverge from the previous module linking representation where a `Component` will try to avoid unnecessary metadata about the component and instead internally only have the bare minimum necessary to instantiate the module. My hope is we can avoid constructing most of the index spaces during instantiation only for it to all ge thrown away. Additionally I'm predicting that we'll need to see through processing where possible to know how to generate adapters and where they are fused. At this time you can't actually call a component's functions, and that's the next PR that I would like to make.
This commit uses the recently updated wasm-tools crates to add tests for the component model added in the previous commit. This involved updating the `wasmtime-wast` crate for component-model changes. Currently the component support there is quite primitive, but enough to at least instantiate components and verify the internals of Wasmtime are all working correctly. Additionally some simple tests for the embedding API have also been added.
05f6f26
to
0af5ff6
Compare
Ok everything is now ready to go here and I think that this is good to land. @fitzgen would you be up for reviewing the tests added here? (sorry I know this is quite late relative to the previous review and this is probably pretty far out of cache for you...) |
Yeah I can take a look tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!
This commit is the first of what will likely be many to implement the
component model proposal in Wasmtime. This will be structured as a
series of incremental commits, most of which haven't been written yet.
My hope is to make this incremental and over time to make this easier to
review and easier to test each step in isolation.
Here much of the skeleton of how components are going to work in
Wasmtime is sketched out. This is not a complete implementation of the
component model so it's not all that useful yet, but some things you can
do are:
with in Wasmtime.
This is already starting to diverge from the previous module linking
representation where a
Component
will try to avoid unnecessarymetadata about the component and instead internally only have the bare
minimum necessary to instantiate the module. My hope is we can avoid
constructing most of the index spaces during instantiation only for it
to all ge thrown away. Additionally I'm predicting that we'll need to
see through processing where possible to know how to generate adapters
and where they are fused.
At this time you can't actually call a component's functions, and that's
the next PR that I would like to make.