diff mbox series

[for-5.2] target/arm: Fix neon VTBL/VTBX for len > 1

Message ID 20201105171126.88014-1-richard.henderson@linaro.org
State New
Headers show
Series [for-5.2] target/arm: Fix neon VTBL/VTBX for len > 1 | expand

Commit Message

Richard Henderson Nov. 5, 2020, 5:11 p.m. UTC
The helper function did not get updated when we reorganized
the vector register file for SVE.  Since then, the neon dregs
are non-sequential and cannot be simply indexed.

At the same time, make the helper function operate on 64-bit
quantities so that we do not have to call it twice.

Fixes: c39c2b9043e
Reported-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/helper.h             |  2 +-
 target/arm/op_helper.c          | 23 +++++++++--------
 target/arm/translate-neon.c.inc | 44 +++++++++++----------------------
 3 files changed, 29 insertions(+), 40 deletions(-)

Comments

Peter Maydell Nov. 9, 2020, 11:59 a.m. UTC | #1
On Thu, 5 Nov 2020 at 17:11, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> The helper function did not get updated when we reorganized
> the vector register file for SVE.  Since then, the neon dregs
> are non-sequential and cannot be simply indexed.
>
> At the same time, make the helper function operate on 64-bit
> quantities so that we do not have to call it twice.
>
> Fixes: c39c2b9043e
> Reported-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> -    val = 0;
> -    for (shift = 0; shift < 32; shift += 8) {
> -        uint32_t index = (ireg >> shift) & 0xff;
> +    for (shift = 0; shift < 64; shift += 8) {
> +        index = (ireg >> shift) & 0xff;
>          if (index < maxindex) {
> -            uint32_t tmp = (table[index >> 3] >> ((index & 7) << 3)) & 0xff;
> -            val |= tmp << shift;
> +            reg = base_reg + (index >> 3);
> +            tmp = env->vfp.zregs[reg >> 1].d[reg & 1];

This can be written
  tmp = *aa32_vfp_dreg(env, reg);
rather than open-coding the conversion from a dreg number into
the appropriate access into the zregs representation.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

so I'll just make that change and apply it to target-arm.next.

thanks
-- PMM
Richard Henderson Nov. 9, 2020, 4:50 p.m. UTC | #2
On 11/9/20 3:59 AM, Peter Maydell wrote:
>> +            tmp = env->vfp.zregs[reg >> 1].d[reg & 1];
> 
> This can be written
>   tmp = *aa32_vfp_dreg(env, reg);
> rather than open-coding the conversion from a dreg number into
> the appropriate access into the zregs representation.

Ah, I forgot about that helper.  Thanks.


r~
diff mbox series

Patch

diff --git a/target/arm/helper.h b/target/arm/helper.h
index 774d2cddb5..ff8148ddc6 100644
--- a/target/arm/helper.h
+++ b/target/arm/helper.h
@@ -245,7 +245,7 @@  DEF_HELPER_FLAGS_2(rsqrte_f32, TCG_CALL_NO_RWG, f32, f32, ptr)
 DEF_HELPER_FLAGS_2(rsqrte_f64, TCG_CALL_NO_RWG, f64, f64, ptr)
 DEF_HELPER_FLAGS_1(recpe_u32, TCG_CALL_NO_RWG, i32, i32)
 DEF_HELPER_FLAGS_1(rsqrte_u32, TCG_CALL_NO_RWG, i32, i32)
-DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i32, i32, i32, ptr, i32)
+DEF_HELPER_FLAGS_4(neon_tbl, TCG_CALL_NO_RWG, i64, env, i32, i64, i64)
 
 DEF_HELPER_3(shl_cc, i32, env, i32, i32)
 DEF_HELPER_3(shr_cc, i32, env, i32, i32)
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index b1065216b2..9bfd6760a0 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -68,21 +68,24 @@  void raise_exception_ra(CPUARMState *env, uint32_t excp, uint32_t syndrome,
     cpu_loop_exit_restore(cs, ra);
 }
 
-uint32_t HELPER(neon_tbl)(uint32_t ireg, uint32_t def, void *vn,
-                          uint32_t maxindex)
+uint64_t HELPER(neon_tbl)(CPUARMState *env, uint32_t desc,
+                          uint64_t ireg, uint64_t def)
 {
-    uint32_t val, shift;
-    uint64_t *table = vn;
+    uint64_t tmp, val = 0;
+    uint32_t maxindex = ((desc & 3) + 1) * 8;
+    uint32_t base_reg = desc >> 2;
+    uint32_t shift, index, reg;
 
-    val = 0;
-    for (shift = 0; shift < 32; shift += 8) {
-        uint32_t index = (ireg >> shift) & 0xff;
+    for (shift = 0; shift < 64; shift += 8) {
+        index = (ireg >> shift) & 0xff;
         if (index < maxindex) {
-            uint32_t tmp = (table[index >> 3] >> ((index & 7) << 3)) & 0xff;
-            val |= tmp << shift;
+            reg = base_reg + (index >> 3);
+            tmp = env->vfp.zregs[reg >> 1].d[reg & 1];
+            tmp = ((tmp >> ((index & 7) << 3)) & 0xff) << shift;
         } else {
-            val |= def & (0xff << shift);
+            tmp = def & (0xffull << shift);
         }
+        val |= tmp;
     }
     return val;
 }
diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc
index 59368cb243..0ae95cb8df 100644
--- a/target/arm/translate-neon.c.inc
+++ b/target/arm/translate-neon.c.inc
@@ -2861,9 +2861,8 @@  static bool trans_VEXT(DisasContext *s, arg_VEXT *a)
 
 static bool trans_VTBL(DisasContext *s, arg_VTBL *a)
 {
-    int n;
-    TCGv_i32 tmp, tmp2, tmp3, tmp4;
-    TCGv_ptr ptr1;
+    TCGv_i64 val, def;
+    TCGv_i32 desc;
 
     if (!arm_dc_feature(s, ARM_FEATURE_NEON)) {
         return false;
@@ -2879,43 +2878,30 @@  static bool trans_VTBL(DisasContext *s, arg_VTBL *a)
         return true;
     }
 
-    n = a->len + 1;
-    if ((a->vn + n) > 32) {
+    if ((a->vn + a->len + 1) > 32) {
         /*
          * This is UNPREDICTABLE; we choose to UNDEF to avoid the
          * helper function running off the end of the register file.
          */
         return false;
     }
-    n <<= 3;
-    tmp = tcg_temp_new_i32();
-    if (a->op) {
-        read_neon_element32(tmp, a->vd, 0, MO_32);
-    } else {
-        tcg_gen_movi_i32(tmp, 0);
-    }
-    tmp2 = tcg_temp_new_i32();
-    read_neon_element32(tmp2, a->vm, 0, MO_32);
-    ptr1 = vfp_reg_ptr(true, a->vn);
-    tmp4 = tcg_const_i32(n);
-    gen_helper_neon_tbl(tmp2, tmp2, tmp, ptr1, tmp4);
 
+    desc = tcg_const_i32((a->vn << 2) | a->len);
+    def = tcg_temp_new_i64();
     if (a->op) {
-        read_neon_element32(tmp, a->vd, 1, MO_32);
+        read_neon_element64(def, a->vd, 0, MO_64);
     } else {
-        tcg_gen_movi_i32(tmp, 0);
+        tcg_gen_movi_i64(def, 0);
     }
-    tmp3 = tcg_temp_new_i32();
-    read_neon_element32(tmp3, a->vm, 1, MO_32);
-    gen_helper_neon_tbl(tmp3, tmp3, tmp, ptr1, tmp4);
-    tcg_temp_free_i32(tmp);
-    tcg_temp_free_i32(tmp4);
-    tcg_temp_free_ptr(ptr1);
+    val = tcg_temp_new_i64();
+    read_neon_element64(val, a->vm, 0, MO_64);
 
-    write_neon_element32(tmp2, a->vd, 0, MO_32);
-    write_neon_element32(tmp3, a->vd, 1, MO_32);
-    tcg_temp_free_i32(tmp2);
-    tcg_temp_free_i32(tmp3);
+    gen_helper_neon_tbl(val, cpu_env, desc, val, def);
+    write_neon_element64(val, a->vd, 0, MO_64);
+
+    tcg_temp_free_i64(def);
+    tcg_temp_free_i64(val);
+    tcg_temp_free_i32(desc);
     return true;
 }