Remove dependency on TargetIsa from Wasmtime crates #3178
Merged
alexcrichton merged 1 commit intobytecodealliance:mainfrom Aug 16, 2021
Merged
Remove dependency on TargetIsa from Wasmtime crates #3178alexcrichton merged 1 commit intobytecodealliance:mainfrom
alexcrichton merged 1 commit intobytecodealliance:mainfrom
Conversation
github-actions
bot
added
cranelift
bjorn3
reviewed
Aug 11, 2021
bjorn3
reviewed
Aug 11, 2021
bjorn3
reviewed
Aug 11, 2021
bjorn3
reviewed
Aug 11, 2021
alexcrichton
force-pushed
the
less-target-isa
branch
3 times, most recently
from
e1eb4e3 to
e65df46
Compare
This commit started off by deleting the `cranelift_codegen::settings` reexport in the `wasmtime-environ` crate and then basically played whack-a-mole until everything compiled again. The main result of this is that the `wasmtime-*` family of crates have generally less of a dependency on the `TargetIsa` trait and type from Cranelift. While the dependency isn't entirely severed yet this is at least a significant start. This commit is intended to be largely refactorings, no functional changes are intended here. The refactorings are: * A `CompilerBuilder` trait has been added to `wasmtime_environ` which server as an abstraction used to create compilers and configure them in a uniform fashion. The `wasmtime::Config` type now uses this instead of cranelift-specific settings. The `wasmtime-jit` crate exports the ability to create a compiler builder from a `CompilationStrategy`, which only works for Cranelift right now. In a cranelift-less build of Wasmtime this is expected to return a trait object that fails all requests to compile. * The `Compiler` trait in the `wasmtime_environ` crate has been souped up with a number of methods that Wasmtime and other crates needed. * The `wasmtime-debug` crate is now moved entirely behind the `wasmtime-cranelift` crate. * The `wasmtime-cranelift` crate is now only depended on by the `wasmtime-jit` crate. * Wasm types in `cranelift-wasm` no longer contain their IR type, instead they only contain the `WasmType`. This is required to get everything to align correctly but will also be required in a future refactoring where the types used by `cranelift-wasm` will be extracted to a separate crate. * I moved around a fair bit of code in `wasmtime-cranelift`. * Some gdb-specific jit-specific code has moved from `wasmtime-debug` to `wasmtime-jit`.
alexcrichton
force-pushed
the
less-target-isa
branch
from
e65df46 to
1c36352
Compare
pchickey
approved these changes
Aug 12, 2021
| style: TableStyle::CallerChecksSignature, | ||
| table: Table { | ||
| wasm_ty: WasmType::FuncRef, | ||
| ty: TableElementType::Func, |
Contributor
There was a problem hiding this comment.
I am grateful that we have gotten rid of all these redundant duplications!
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
3 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 started off by deleting the
cranelift_codegen::settingsreexport in the
wasmtime-environcrate and then basically playedwhack-a-mole until everything compiled again. The main result of this is
that the
wasmtime-*family of crates have generally less of adependency on the
TargetIsatrait and type from Cranelift. While thedependency isn't entirely severed yet this is at least a significant
start.
This commit is intended to be largely refactorings, no functional
changes are intended here. The refactorings are:
A
CompilerBuildertrait has been added towasmtime_environwhichserver as an abstraction used to create compilers and configure them
in a uniform fashion. The
wasmtime::Configtype now uses thisinstead of cranelift-specific settings. The
wasmtime-jitcrateexports the ability to create a compiler builder from a
CompilationStrategy, which only works for Cranelift right now. In acranelift-less build of Wasmtime this is expected to return a trait
object that fails all requests to compile.
The
Compilertrait in thewasmtime_environcrate has been soupedup with a number of methods that Wasmtime and other crates needed.
The
wasmtime-debugcrate is now moved entirely behind thewasmtime-craneliftcrate.The
wasmtime-craneliftcrate is now only depended on by thewasmtime-jitcrate.Wasm types in
cranelift-wasmno longer contain their IR type,instead they only contain the
WasmType. This is required to geteverything to align correctly but will also be required in a future
refactoring where the types used by
cranelift-wasmwill be extractedto a separate crate.
I moved around a fair bit of code in
wasmtime-cranelift.Some gdb-specific jit-specific code has moved from
wasmtime-debugtowasmtime-jit.Note that this is built on #3176, but I wanted to keep all these refactorings ideally bite-ish sized to be more easily digestable