Wasmtime GitHub notifications bot (Feb 17 2023 at 17:11): Wasmtime GitHub notifications bot (Feb 17 2023 at 17:11):alexcrichton opened PR #5819 from more-avx to main:
This commit goes through
inst.isleand adds a corresponding AVX lowering for most SSE lowerings. I opted to skip instructions where the SSE lowering didn't read/modify a register, such asroundps. I think that AVX will benefit these instructions when there's load-merging since AVX doesn't require alignment, but I've deferred that work to a future PR.Otherwise though in this PR I think all (or almost all) of the 3-operand forms of AVX instructions are supported with their SSE counterparts. This should ideally improve codegen slightly by removing register pressure and the need for
movdqabetween registers. I've attempted to ensure that there's at least one codegen test for all the new instructions.As a side note, the recent capstone integration into
precise-outputtests helped me catch a number of encoding bugs much earlier than otherwise, so I've found that incredibly useful in tests!<!--
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 (Feb 17 2023 at 18:08):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:08):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:08):cfallin created PR review comment:
I'm not wild about the argument-dependent regalloc metadata shape here, and ideally I'd prefer us to split out variants like
XmmRmiRVexConstthat are used instead whenever the intent of an instruction is to produce a constant 0/1/whatever, and that assert the correct setup for that; but that's a separate refactor you don't necessarily need to do in this PR. Just noting as I see the existing pattern expanding...
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:08):cfallin created PR review comment:
Given that, could we split this into two enum variants, one with
XmmMemand one withGprMem, to keep the type safety?
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:51):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:51):alexcrichton created PR review comment:
Sure, I was following this precedent but I can split out too.
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:51):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Feb 17 2023 at 18:51):alexcrichton created PR review comment:
Sure yeah, I was just following the preexisting pattern for the xmm instructions, but I can try to update those too in a subsequent PR.
Wasmtime GitHub notifications bot (Feb 17 2023 at 21:50):alexcrichton updated PR #5819 from more-avx to main.
Wasmtime GitHub notifications bot (Feb 17 2023 at 22:02):alexcrichton updated PR #5819 from more-avx to main.
Wasmtime GitHub notifications bot (Feb 17 2023 at 22:51):cfallin submitted PR review.
Wasmtime GitHub notifications bot (Feb 17 2023 at 23:28):alexcrichton updated PR #5819 from more-avx to main.
Wasmtime GitHub notifications bot (Feb 20 2023 at 16:19):alexcrichton merged PR #5819.
Last updated: Oct 23 2024 at 20:03 UTC