Re: [Qemu-devel] [PATCH v5 18/35] target/arm: Implement SVE Floating Point Multiply Indexed Group

[Peter Maydell]

On 21 June 2018 at 02:53, Richard Henderson
<address@hidden> wrote:
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper.h        | 14 +++++++++++
>  target/arm/translate-sve.c | 50 ++++++++++++++++++++++++++++++++++++++
>  target/arm/vec_helper.c    | 48 ++++++++++++++++++++++++++++++++++++
>  target/arm/sve.decode      | 19 +++++++++++++++
>  4 files changed, 131 insertions(+)

> +static bool trans_FMLA_zzxz(DisasContext *s, arg_FMLA_zzxz *a, uint32_t insn)
> +{
> +    static gen_helper_gvec_4_ptr * const fns[3] = {
> +        gen_helper_gvec_fmla_idx_h,
> +        gen_helper_gvec_fmla_idx_s,
> +        gen_helper_gvec_fmla_idx_d,
> +    };
> +
> +    if (sve_access_check(s)) {
> +        unsigned vsz = vec_full_reg_size(s);
> +        TCGv_ptr status = get_fpstatus_ptr(a->esz == MO_16);
> +        tcg_gen_gvec_4_ptr(vec_full_reg_offset(s, a->rd),
> +                           vec_full_reg_offset(s, a->rn),
> +                           vec_full_reg_offset(s, a->rm),
> +                           vec_full_reg_offset(s, a->ra),
> +                           status, vsz, vsz, a->index * 2 + a->sub,

Why are we multiplying the index by 2 here? Are we just encoding
(index, sub) into a constant to pull out again the other side?
If so, comment might help. (Also I find shifts and ors make it
clearer that we're just shifting values around; multiplication
and addition to me implies that we're really doing arithmetic.)

> +                           fns[a->esz - 1]);
> +        tcg_temp_free_ptr(status);
> +    }
> +    return true;
> +}

Otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM