Re: [PATCH v2 24/45] target/arm: Implement fp16 for Neon VRECPE, VRSQRTE using gvec

[Richard Henderson]

On 8/28/20 11:33 AM, Peter Maydell wrote:
> We already have gvec helpers for floating point VRECPE and
> VRQSRTE, so convert the Neon decoder to use them and
> add the fp16 support.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/translate-neon.c.inc | 29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/target/arm/translate-neon.c.inc b/target/arm/translate-neon.c.inc
> index 9d0959517fa..872f093a1fc 100644
> --- a/target/arm/translate-neon.c.inc
> +++ b/target/arm/translate-neon.c.inc
> @@ -3857,13 +3857,38 @@ static bool do_2misc_fp(DisasContext *s, arg_2misc *a,
>          return do_2misc_fp(s, a, FUNC);                         \
>      }
>  
> -DO_2MISC_FP(VRECPE_F, gen_helper_recpe_f32)
> -DO_2MISC_FP(VRSQRTE_F, gen_helper_rsqrte_f32)
>  DO_2MISC_FP(VCVT_FS, gen_helper_vfp_sitos)
>  DO_2MISC_FP(VCVT_FU, gen_helper_vfp_uitos)
>  DO_2MISC_FP(VCVT_SF, gen_helper_vfp_tosizs)
>  DO_2MISC_FP(VCVT_UF, gen_helper_vfp_touizs)
>  
> +#define DO_2MISC_FP_VEC(INSN, HFUNC, SFUNC)                             \
> +    static void gen_##INSN(unsigned vece, uint32_t rd_ofs,              \
> +                           uint32_t rm_ofs,                             \
> +                           uint32_t oprsz, uint32_t maxsz)              \
> +    {                                                                   \
> +        static gen_helper_gvec_2_ptr * const fns[4] = {                 \
> +            NULL, HFUNC, SFUNC, NULL,                                   \
> +        };                                                              \
> +        TCGv_ptr fpst;                                                  \
> +        fpst = fpstatus_ptr(vece == 1 ? FPST_STD_F16 : FPST_STD);       \

Perhaps clearer with MO_16 instead of 1.

> +        tcg_gen_gvec_2_ptr(rd_ofs, rm_ofs, fpst, oprsz, maxsz, 0,       \
> +                           fns[vece]);                                  \
> +        tcg_temp_free_ptr(fpst);                                        \
> +    }                                                                   \
> +    static bool trans_##INSN(DisasContext *s, arg_2misc *a)             \
> +    {                                                                   \
> +        if (a->size == 0 ||                                             \
> +            (a->size == 1 && !dc_isar_feature(aa32_fp16_arith, s)))     \
> +        {                                                               \

Likewise, and the { is on the wrong line.

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

r~