Support specifying stack slot alignments #6716
Comments
|
Also necessary to fix rust-lang/rustc_codegen_cranelift#1258. |
cfallin
added a commit
to cfallin/wasmtime
that referenced
this issue
May 16, 2024
Fixes bytecodealliance#6716. Currently, stack slots on the stack are aligned only to a machine-word boundary. This is insufficient for some use-cases: for example, storing SIMD data or structs that require a larger alignment. This PR adds a parameter to the `StackSlotData` to specify alignment, and the associated logic to the CLIF parser and printer. It updates the shared ABI code to compute the stackslot layout taking the alignment into account. In order to ensure the alignment is always a power of two, it is stored as a shift amount (log2 of actual alignment) in the IR.
cfallin
added a commit
to cfallin/wasmtime
that referenced
this issue
May 16, 2024
Fixes bytecodealliance#6716. Currently, stack slots on the stack are aligned only to a machine-word boundary. This is insufficient for some use-cases: for example, storing SIMD data or structs that require a larger alignment. This PR adds a parameter to the `StackSlotData` to specify alignment, and the associated logic to the CLIF parser and printer. It updates the shared ABI code to compute the stackslot layout taking the alignment into account. In order to ensure the alignment is always a power of two, it is stored as a shift amount (log2 of actual alignment) in the IR.
github-merge-queue bot
pushed a commit
that referenced
this issue
May 16, 2024
* Cranelift: add alignment parameter to stack slots. Fixes #6716. Currently, stack slots on the stack are aligned only to a machine-word boundary. This is insufficient for some use-cases: for example, storing SIMD data or structs that require a larger alignment. This PR adds a parameter to the `StackSlotData` to specify alignment, and the associated logic to the CLIF parser and printer. It updates the shared ABI code to compute the stackslot layout taking the alignment into account. In order to ensure the alignment is always a power of two, it is stored as a shift amount (log2 of actual alignment) in the IR. * Apply suggestions from code review Co-authored-by: Trevor Elliott <[email protected]> * Update filetest. * Update alignment to ValRaw vector. * Fix printer test. * cargo-fmt from suggestion update. --------- Co-authored-by: Trevor Elliott <[email protected]>
|
Would you mind reopening this? It hasn't been fully resolved yet: https://github.com/bytecodealliance/wasmtime/pull/8635/files#r1603954465 |
|
Sure; to summarize here, the issue is that alignment is now correct with respect to the start of stack frame, but start of stack frame is only aligned as per ABI (e.g. 16 bytes on x86-64 and aarch64) so we need to dynamically align SP (it must be dynamic, we can't know statically anything more than the ABI guarantee). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Feature
See title.
Benefit
Correct alignment are necessary to avoid crashes on some architectures and code may depend on correct alignment. In addition rustc checks that the right alignment is used when dereferencing a raw pointer in debug mode. This breaks rayon which allocates a stack value with cacheline alignment and then takes a raw pointer which it later dereferences. See rust-lang/rustc_codegen_cranelift#1381.
Implementation
Sort stackslots by alignment for better packing and then add padding as necessary and if the alignment of a stack slot exceeds the ABI stack alignment realign the stack at runtime.
Alternatives
Doing dynamic alignment at runtime in the cranelift ir producer. This is slower and has higher stack usage overhead.
The text was updated successfully, but these errors were encountered: