[for-6.2,03/53] target/arm: Fix MVE VSLI by 0 and VSRI by <dt>

Message ID 20210729111512.16541-4-peter.maydell@linaro.org
State Superseded
Headers show
Series
  • target/arm: MVE slices 3 and 4
Related show

Commit Message

Peter Maydell July 29, 2021, 11:14 a.m.
In the MVE shift-and-insert insns, we special case VSLI by 0
and VSRI by <dt>. VSRI by <dt> means "don't update the destination",
which is what we've implemented. However VSLI by 0 is "set
destination to the input", so we don't want to use the same
special-casing that we do for VSRI by <dt>.

Since the generic logic gives the right answer for a shift
by 0, just use that.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 target/arm/mve_helper.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.20.1

Comments

Richard Henderson July 30, 2021, 6:56 p.m. | #1
On 7/29/21 1:14 AM, Peter Maydell wrote:
> In the MVE shift-and-insert insns, we special case VSLI by 0

> and VSRI by <dt>. VSRI by <dt> means "don't update the destination",

> which is what we've implemented. However VSLI by 0 is "set

> destination to the input", so we don't want to use the same

> special-casing that we do for VSRI by <dt>.

> 

> Since the generic logic gives the right answer for a shift

> by 0, just use that.

> 

> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>

> ---

>   target/arm/mve_helper.c | 9 +++++----

>   1 file changed, 5 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Patch

diff --git a/target/arm/mve_helper.c b/target/arm/mve_helper.c
index db5d6220854..f14fa914b68 100644
--- a/target/arm/mve_helper.c
+++ b/target/arm/mve_helper.c
@@ -1279,11 +1279,12 @@  DO_2SHIFT_S(vrshli_s, DO_VRSHLS)
         uint16_t mask;                                                  \
         uint64_t shiftmask;                                             \
         unsigned e;                                                     \
-        if (shift == 0 || shift == ESIZE * 8) {                         \
+        if (shift == ESIZE * 8) {                                       \
             /*                                                          \
-             * Only VSLI can shift by 0; only VSRI can shift by <dt>.   \
-             * The generic logic would give the right answer for 0 but  \
-             * fails for <dt>.                                          \
+             * Only VSRI can shift by <dt>; it should mean "don't       \
+             * update the destination". The generic logic can't handle  \
+             * this because it would try to shift by an out-of-range    \
+             * amount, so special case it here.                         \
              */                                                         \
             goto done;                                                  \
         }                                                               \