Start compiling module-linking modules #2093
Merged
alexcrichton merged 1 commit intobytecodealliance:mainfrom Nov 6, 2020
Merged
Start compiling module-linking modules #2093alexcrichton merged 1 commit intobytecodealliance:mainfrom
alexcrichton merged 1 commit intobytecodealliance:mainfrom
Conversation
11 tasks
github-actions
bot
added
cranelift
Subscribe to Label Actioncc @bnjbvr, @fitzgen, @peterhuene DetailsThis issue or pull request has been labeled: "cranelift", "cranelift:wasm", "fuzzing", "lightbeam", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
alexcrichton
force-pushed
the
compile-nested-modules
branch
2 times, most recently
from
b589e70 to
409b962
Compare
Member
Author
|
Ok I've rebased this now and I think it should be ready for review! |
fitzgen
approved these changes
Nov 5, 2020
| WasmFeatures::default() | ||
| } | ||
|
|
||
| /// Indicates that this module will have `amount` submodules. |
Member
There was a problem hiding this comment.
So this only includes nested submodules, not the top level module? Good to make this explicit in the comment.
Member
Author
There was a problem hiding this comment.
Ah good point, but yes, this only includes the top-level module's modules (or whatever the current module is), basically just "we found the module section in the wasm file"
alexcrichton
force-pushed
the
compile-nested-modules
branch
from
409b962 to
104cde1
Compare
This commit is intended to be the first of many in implementing the module linking proposal. At this time this builds on bytecodealliance#2059 so it shouldn't land yet. The goal of this commit is to compile bare-bones modules which use module linking, e.g. those with nested modules. My hope with module linking is that almost everything in wasmtime only needs mild refactorings to handle it. The goal is that all per-module structures are still per-module and at the top level there's just a `Vec` containing a bunch of modules. That's implemented currently where `wasmtime::Module` contains `Arc<[CompiledModule]>` and an index of which one it's pointing to. This should enable serialization/deserialization of any module in a nested modules scenario, no matter how you got it. Tons of features of the module linking proposal are missing from this commit. For example instantiation flat out doesn't work, nor does import/export of modules or instances. That'll be coming as future commits, but the purpose here is to start laying groundwork in Wasmtime for handling lots of modules in lots of places.
alexcrichton
force-pushed
the
compile-nested-modules
branch
from
104cde1 to
bbc93ad
Compare
cfallin
pushed a commit
that referenced
this pull request
Nov 30, 2020
This commit is intended to be the first of many in implementing the module linking proposal. At this time this builds on #2059 so it shouldn't land yet. The goal of this commit is to compile bare-bones modules which use module linking, e.g. those with nested modules. My hope with module linking is that almost everything in wasmtime only needs mild refactorings to handle it. The goal is that all per-module structures are still per-module and at the top level there's just a `Vec` containing a bunch of modules. That's implemented currently where `wasmtime::Module` contains `Arc<[CompiledModule]>` and an index of which one it's pointing to. This should enable serialization/deserialization of any module in a nested modules scenario, no matter how you got it. Tons of features of the module linking proposal are missing from this commit. For example instantiation flat out doesn't work, nor does import/export of modules or instances. That'll be coming as future commits, but the purpose here is to start laying groundwork in Wasmtime for handling lots of modules in lots of places.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This commit is intended to be the first of many in implementing the
module linking proposal. At this time this builds on #2059 so it
shouldn't land yet. The goal of this commit is to compile bare-bones
modules which use module linking, e.g. those with nested modules.
My hope with module linking is that almost everything in wasmtime only
needs mild refactorings to handle it. The goal is that all per-module
structures are still per-module and at the top level there's just a
Veccontaining a bunch of modules. That's implemented currently wherewasmtime::ModulecontainsArc<[CompiledModule]>and an index ofwhich one it's pointing to. This should enable
serialization/deserialization of any module in a nested modules
scenario, no matter how you got it.
Tons of features of the module linking proposal are missing from this
commit. For example instantiation flat out doesn't work, nor does
import/export of modules or instances. That'll be coming as future
commits, but the purpose here is to start laying groundwork in Wasmtime
for handling lots of modules in lots of places.