Re: [Qemu-devel] [PATCH v2 06/11] target/arm: Decode aa32 armv8.1 two reg and a scalar

[Peter Maydell]

On 18 December 2017 at 17:24, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate.c | 38 +++++++++++++++++++++++++++++++++++---
>  1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index a9587ae242..1a0b0eaced 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -6973,11 +6973,43 @@ static int disas_neon_data_insn(DisasContext *s, 
> uint32_t insn)
>                          }
>                          neon_store_reg64(cpu_V0, rd + pass);
>                      }
> +                    break;
> +                case 14: /* VQRDMLAH scalar */
> +                case 15: /* VQRDMLSH scalar */
> +                    if (!arm_dc_feature(s, ARM_FEATURE_V8_1_SIMD)) {
> +                        return 1;
> +                    }
> +                    if (u && ((rd | rn) & 1)) {
> +                        return 1;
> +                    }

The pseudocode also has UNDEF if Q==1 && Vm<0> == 1 -- have we
already done that test earlier? I can't see it, but our neon decode
code is quite hard to read...

> +                    tmp2 = neon_get_scalar(size, rm);
> +                    for (pass = 0; pass < (u ? 4 : 2); pass++) {
> +                        void (*fn)(TCGv_i32, TCGv_env, TCGv_i32,
> +                                   TCGv_i32, TCGv_i32);

Can we define a typedef for this, please ?

>
> -
> +                        tmp = neon_load_reg(rn, pass);
> +                        tmp3 = neon_load_reg(rd, pass);
> +                        if (op == 14) {
> +                            if (size == 1) {
> +                                fn = gen_helper_neon_qrdmlah_s16;
> +                            } else {
> +                                fn = gen_helper_neon_qrdmlah_s32;
> +                            }
> +                        } else {
> +                            if (size == 1) {
> +                                fn = gen_helper_neon_qrdmlsh_s16;
> +                            } else {
> +                                fn = gen_helper_neon_qrdmlsh_s32;
> +                            }
> +                        }
> +                        fn(tmp, cpu_env, tmp, tmp2, tmp3);
> +                        tcg_temp_free_i32(tmp3);
> +                        neon_store_reg(rd, pass, tmp);
> +                    }
> +                    tcg_temp_free_i32(tmp2);
>                      break;
> -                default: /* 14 and 15 are RESERVED */
> -                    return 1;
> +                default:
> +                    g_assert_not_reached();
>                  }
>              }
>          } else { /* size == 3 */
> --
> 2.14.3

Otherwise

Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM