Wasmtime GitHub notifications bot (Jan 30 2023 at 15:51): Wasmtime GitHub notifications bot (Jan 30 2023 at 15:51):saulecabrera opened PR #5652 from winch-codegen-use-aarch64-backend to main:
This patch introduces basic Aarch64 code generation by using
cranelift-codegen's backend.This change does not:
- Change the semantics of the code generation
- Adds support for other Wasm instructions
The most notable change in this patch is how addressing modes are handled at the MacroAssembler layer: instead of having a canonical address representation, this patch introduces the addressing mode as an associated type in the MacroAssembler trait. This approach has the advantage that gives each ISA enough flexiblity to describe the addressing modes and their constraints in isolation without having to worry on how a particular addressing mode is going to affect other ISAs. In the case of Aarch64 this becomes useful to describe indexed addressing modes (particularly from the stack pointer).
This patch uses the concept of a shadow stack pointer (x28) as a workaround to Aarch64's stack pointer 16-byte alignment. This constraint is enforced by:
Introducing specialized addressing modes when using the real stack pointer; this enables auditing when the real stack pointer is used. As of this change, the real stack pointer is only used in the function's prologue and epilogue.
Asserting that the real stack pointer is not used as a base for addressing modes.
Ensuring that at any point during the code generation process where the stack pointer changes (e.g. when stack space is allocated / deallocated) the value of the real stack pointer is copied into the shadow stack pointer.
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
Wasmtime GitHub notifications bot (Jan 30 2023 at 15:51):saulecabrera requested cfallin for a review on PR #5652.
Wasmtime GitHub notifications bot (Jan 30 2023 at 15:57):saulecabrera updated PR #5652 from winch-codegen-use-aarch64-backend to main.
Wasmtime GitHub notifications bot (Jan 30 2023 at 15:59):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Jan 30 2023 at 15:59):bjorn3 created PR review comment:
You may want to call
.set_skipdata(true)on the built disassembler if you want to use inline constant pools. Otherwise capstone will stop disassembling at the first instruction it can't disassemble (eg due to being a constant pool)
Wasmtime GitHub notifications bot (Jan 31 2023 at 01:42):saulecabrera updated PR #5652 from winch-codegen-use-aarch64-backend to main.
Wasmtime GitHub notifications bot (Jan 31 2023 at 18:56):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jan 31 2023 at 18:56):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Jan 31 2023 at 18:56):cfallin created PR review comment:
Since this variant only supports pre-/post-indexed forms (i.e., forms that update
SP), maybe we can name itSPOffsetWithUpdate(or something less cumbersome, though nothing comes to mind at the moment...)?
Wasmtime GitHub notifications bot (Jan 31 2023 at 18:56):cfallin created PR review comment:
Could we add a block comment somewhere describing what the shadow stack is and how it works? (Or maybe I missed it earlier in which case let's have a reference to docs here!)
Wasmtime GitHub notifications bot (Jan 31 2023 at 18:56):cfallin created PR review comment:
Ah, here's a good description, thanks!
A few questions on invariants:
- Is
x28now a special part of the ABI, i.e., neither caller- nor callee-save but an implicit arg in and out? I guess calls out to host code are safe becausex28is normally a callee-save, so it'll be preserved. Do we need to set it before calling in from the host though?- Stating a bit more about the update strategy (when to copy sp to shadow-sp, do we ever need to copy in the other direction?, etc.) would be good here
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:54):saulecabrera updated PR #5652 from winch-codegen-use-aarch64-backend to main.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:55):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:55):saulecabrera created PR review comment:
I renamed this to
IndexedSPOffset, let me know if that's clear enough. Else I'm happy to find a better name.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:56):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:56):saulecabrera created PR review comment:
I'd like to keep the documentation of the
shadow_spatregs::shadow_sp(), so I've added a reference here.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:57):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Feb 01 2023 at 16:57):saulecabrera created PR review comment:
I added more details to the documentation here addressing your questions around the invariants also taking as a base our conversation during our meeting.
Wasmtime GitHub notifications bot (Feb 01 2023 at 17:01):saulecabrera updated PR #5652 from winch-codegen-use-aarch64-backend to main.
Wasmtime GitHub notifications bot (Feb 02 2023 at 12:24):saulecabrera requested cfallin for a review on PR #5652.
Wasmtime GitHub notifications bot (Feb 02 2023 at 22:23):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Feb 02 2023 at 22:24):cfallin merged PR #5652.
Last updated: Dec 23 2024 at 12:05 UTC