saulecabrera opened PR #5691 from winch-x64-cleanups
to main
:
This commit contains a small set of clean up items for x64.
Notably:
- Adds filetests
- Documents why 16 for the arg base offset abi implementation, for clarity.
- Fixes a bug in the spill implementation that was catched while anlyzing the filetests results. The fix consists of emitting a load instead of a store into the scratch register before spiiling its value.
- Remove dead code for pretty printing registers which is not needed anymore since we now have proper disassembly.
<!--
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.
-->
saulecabrera requested cfallin for a review on PR #5691.
saulecabrera submitted PR review.
saulecabrera submitted PR review.
saulecabrera created PR review comment:
This path is not heavily exercised at the moment. I'll add more extensive tests once we support
pop
at the Masm layer, which should be once we support all the binary/unary operations since we'd be able to test more complex programs.
saulecabrera updated PR #5691 from winch-x64-cleanups
to main
.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
I just realized here that x64 also keeps
rsp
only 8-aligned -- this is fine for accesses to the stack in the function body (unlike aarch64) but will also need alignment when calls are implemented, because the x64 SysV ABI requires 16-alignment on call. (Function prologues can rely on this by using aligned SIMD stores...) Possible you're already thinking of this, but just wanted to note!
cfallin has enabled auto merge for PR #5691.
cfallin merged PR #5691.
saulecabrera edited PR #5691 from winch-x64-cleanups
to main
:
This commit contains a small set of clean up items for x64.
Notably:
- Adds filetests
- Documents why 16 for the arg base offset abi implementation, for clarity.
- Fixes a bug in the spill implementation that was caught while analyzing the filetests results. The fix consists of emitting a load instead of a store into the scratch register before spilling its value.
- Remove dead code for pretty printing registers which is not needed anymore since we now have proper disassembly.
<!--
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.
-->
Last updated: Nov 22 2024 at 16:03 UTC