riscv64: Implement a few SIMD arithmetic ops #6268
Conversation
github-actions
bot
added
cranelift
These were accidentally reversed from what we declare in the isle emit helper
Looks like x86 does not implement it
afonso360
force-pushed
the
riscv-vec-arithmetic
branch
from
c6eeba1
to
09447be
Compare
| VecAluOpRRR::Vadd => write!(f, "vadd.vv"), | ||
| VecAluOpRRR::Vsub => write!(f, "vsub.vv"), | ||
| VecAluOpRRR::Vmul => write!(f, "vmul.vv"), | ||
| VecAluOpRRR::Vmulh => write!(f, "vmulh.vv"), | ||
| VecAluOpRRR::Vmulhu => write!(f, "vmulhu.vv"), | ||
| VecAluOpRRR::Vand => write!(f, "vand.vv"), | ||
| VecAluOpRRR::Vor => write!(f, "vor.vv"), | ||
| VecAluOpRRR::Vxor => write!(f, "vxor.vv"), |
There was a problem hiding this comment.
If these all follow the same pattern, one thing I found helpful for AVX was do do something like:
let mut s = format!("{self:?}");
s.make_ascii_lowercase();
s.push_str(".vv");
f.write_str(&s)which can cut down on codegen times and additionally make this a bit easier to maintain. If the instructions have all sorts of different names though this may not work out well
There was a problem hiding this comment.
We also have another variant for all of these, which is v*.vx. However, it should still be fairly easy to fit into that scheme when we do need it.
Edit: Hmm, It might not be so easy, because I was planning on renaming these enum arms into VaddVV and VaddVX which would then break that. But I don't mind doing it this way for now and figuring it out later.
| target aarch64 | ||
| target s390x | ||
| set enable_simd | ||
| target x86_64 |
There was a problem hiding this comment.
Mind copying over the target x86_64 has_sse41=false line as well from the original test? (maybe lost through a rebase by accident)
There was a problem hiding this comment.
This also makes me idly think that perhaps there should be a ; skip: riscv directive for filetests or similar, but not something to be added in this PR of course.
* riscv64: Swap order of `VecAluRRR` source registers
These were accidentally reversed from what we declare in the isle emit helper
* riscv64: Add SIMD `isub`
* riscv64: Add SIMD `imul`
* riscv64: Add `{u,s}mulhi`
* riscv64: Add `b{and,or,xor}`
* cranelift: Move `imul.i8x16` runtest to separate file
Looks like x86 does not implement it
* riscv64: Better formatting for `VecAluOpRRR`
* cranelift: Enable x86 SIMD tests with `has_sse41=false`
馃憢 Hey,
This PR Implements a few more arithmetic ops, I didn't want to implement too many since I want to get the wasmtime testsuite working to get better test coverage.
Similarly to #6266, I've also switched the scalar rules to match only scalars, since most of them used
fits_in_64which also matches vectors.I also had accidentally switched the order of the registers in
VecAluRRR, which I only noticed when implementingisub.Implemented ops are:
isubimulsmulhiumulhibandborbxorThis PR is based on #6266, I'll rebase when that lands.