dheaton-arm opened PR #3268 from implement-swizzle-splat
to main
:
Implemented for the Cranelift interpreter:
Swizzle
to shuffle ani8x16
SIMD vector based
on the indices specified in another vector of the same size.
Splat
to create a SIMD vector with all lanes having the same value.Copyright (c) 2021, Arm Limited
<!--
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.
-->
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Could you return an error in case of going out-of-bounds?
dheaton-arm submitted PR review.
dheaton-arm created PR review comment:
In the docs, it's specified that the resulting lane should just be 0 for indices out of bounds - unless that's incorrect?
bjorn3 submitted PR review.
bjorn3 created PR review comment:
Didn't know that. https://github.com/bytecodealliance/wasmtime/blob/16854e73c5bcd3323a51214b267f628ac41c9607/cranelift/codegen/src/isa/x64/lower.rs#L6339-L6343
abrown submitted PR review.
abrown submitted PR review.
abrown created PR review comment:
I'm slightly surprised that I allowed swizzles for types other than
i8x16
(here). In the spirit of constraining the instruction set, this might be another place to consider pruning: it is a bit strange that one can send in ani8x16
, rearrange it with the swizzle mask, and then have some other vector type. Any thoughts on this? @cfallin, @akirilov-arm, @jlb6740?(In any case, this PR should merge as-is and we can remove these later if that's what we decide--thanks for finding this!).
abrown created PR review comment:
nit: when visually checking these values, it would be more helpful if they were in hex format so we could distinguish what lanes have what values.
abrown edited PR review comment.
afonso360 submitted PR review.
afonso360 created PR review comment:
We can fix the old backend errors by replacing the
target x86_64
withtarget x86_64 machinst
.This causes it to be ignored on the old style x86 backend.
abrown submitted PR review.
abrown created PR review comment:
Perfect, let's do that then.
afonso360 submitted PR review.
dheaton-arm updated PR #3268 from implement-swizzle-splat
to main
.
dheaton-arm updated PR #3268 from implement-swizzle-splat
to main
.
cfallin submitted PR review.
cfallin created PR review comment:
Agreed!
cfallin submitted PR review.
cfallin created PR review comment:
Yes, in principle I agree; I think we should be sort of systematic about this in a pass over all instructions, thinking about what types make sense, so I don't think we need to make an issue just for this, or adjust the test right now, but noted!
dheaton-arm updated PR #3268 from implement-swizzle-splat
to main
.
abrown merged PR #3268.
Last updated: Nov 22 2024 at 16:03 UTC