cranelift: Expose MachLabel#6529
Merged
cfallin merged 1 commit intobytecodealliance:mainfrom Jun 6, 2023
Merged
Conversation
This is the first of a series of patches to support control flow in Winch. This change exposes `MachLabel` from cranelift for it to be consumed by Winch's `MacroAssembler` and `Assembler`.
github-actions
bot
added
the
cranelift
cfallin
approved these changes
Jun 6, 2023
saulecabrera
added a commit
to saulecabrera/wasmtime
that referenced
this pull request
Feb 8, 2024
Closes bytecodealliance#6529 This patch adds support for all the instructions involving WebAssembly loads and stores for 32-bit memories. Given that the `memory64` proposal is not enabled by default, this patch doesn't include an implementation/tests for it; in theory minimal tweaks to the currrent implementation will be needed in order to support 64-bit memories. Implemenation-wise, this change, follows a similar pattern as Cranelift in order to calculate addresses for dynamic/static heaps, the main difference being that in some cases, doing less work at compile time is preferred; the current implemenation only checks for the general case of out-of-bounds access for dynamic heaps for example. Another important detail regarding the implementation, is the introduction of `MacroAssembler::wasm_load` and `MacroAssembler::wasm_store`, which internally use a common implemenation for loads and stores, with the only difference that the `wasm_*` variants set the right flags in order to signal that these operations are not trusted and might trap. Finally, given that this change introduces support for the last set of instructions missing for a Wasm MVP, it removes most of Winch's copy of the spectest suite, and switches over to using the official test suite where possible (for tests that don't use SIMD or Reference Types). Follow-up items: * Before doing any deep benchmarking I'm planning on landing a couple of improvements regarding compile times that I've identified in parallel to this change. * The `imports.wast` tests are disabled because I've identified a bug with `call_indirect`, which is not related to this change and exists in main. * Find a way to run the `tests/all/memory.rs` (or perhaps most of integration tests) with Winch. -- prtest:full
github-merge-queue bot
pushed a commit
that referenced
this pull request
Feb 9, 2024
* winch: Add support for WebAssembly loads/stores Closes #6529 This patch adds support for all the instructions involving WebAssembly loads and stores for 32-bit memories. Given that the `memory64` proposal is not enabled by default, this patch doesn't include an implementation/tests for it; in theory minimal tweaks to the currrent implementation will be needed in order to support 64-bit memories. Implemenation-wise, this change, follows a similar pattern as Cranelift in order to calculate addresses for dynamic/static heaps, the main difference being that in some cases, doing less work at compile time is preferred; the current implemenation only checks for the general case of out-of-bounds access for dynamic heaps for example. Another important detail regarding the implementation, is the introduction of `MacroAssembler::wasm_load` and `MacroAssembler::wasm_store`, which internally use a common implemenation for loads and stores, with the only difference that the `wasm_*` variants set the right flags in order to signal that these operations are not trusted and might trap. Finally, given that this change introduces support for the last set of instructions missing for a Wasm MVP, it removes most of Winch's copy of the spectest suite, and switches over to using the official test suite where possible (for tests that don't use SIMD or Reference Types). Follow-up items: * Before doing any deep benchmarking I'm planning on landing a couple of improvements regarding compile times that I've identified in parallel to this change. * The `imports.wast` tests are disabled because I've identified a bug with `call_indirect`, which is not related to this change and exists in main. * Find a way to run the `tests/all/memory.rs` (or perhaps most of integration tests) with Winch. -- prtest:full * Review comments
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 is the first of a series of patches to support control flow in Winch.
This change exposes
MachLabelfrom cranelift for it to be consumed by Winch'sMacroAssemblerandAssembler.