Re: [PATCH 2/2] target/arm: Use tcg_gen_gvec_5_ptr for sve FMLA/FCMLA

[Alex Bennée]

Richard Henderson <address@hidden> writes:

> Now that we can pass 7 parameters, do not encode register
> operands within simd_data.

What defines the upper limit? Is it the ABI of the backend or just the
efficiency of implementing the prologue for the call?
> --- a/target/arm/sve_helper.c
> +++ b/target/arm/sve_helper.c
> @@ -3372,23 +3372,11 @@ DO_ZPZ_FP(sve_ucvt_dd, uint64_t,     , 
> uint64_to_float64)
>  
>  #undef DO_ZPZ_FP
>  
> -/* 4-operand predicated multiply-add.  This requires 7 operands to pass
> - * "properly", so we need to encode some of the registers into DESC.
> - */
> -QEMU_BUILD_BUG_ON(SIMD_DATA_SHIFT + 20 > 32);
> -
> -static void do_fmla_zpzzz_h(CPUARMState *env, void *vg, uint32_t desc,
> +static void do_fmla_zpzzz_h(void *vd, void *vn, void *vm, void *va, void *vg,
> +                            float_status *status, uint32_t desc,
>                              uint16_t neg1, uint16_t neg3)
>  {
>      intptr_t i = simd_oprsz(desc);
> -    unsigned rd = extract32(desc, SIMD_DATA_SHIFT, 5);
> -    unsigned rn = extract32(desc, SIMD_DATA_SHIFT + 5, 5);
> -    unsigned rm = extract32(desc, SIMD_DATA_SHIFT + 10, 5);
> -    unsigned ra = extract32(desc, SIMD_DATA_SHIFT + 15, 5);
> -    void *vd = &env->vfp.zregs[rd];
> -    void *vn = &env->vfp.zregs[rn];
> -    void *vm = &env->vfp.zregs[rm];
> -    void *va = &env->vfp.zregs[ra];
<snip>
> @@ -4010,25 +4001,14 @@ static bool trans_FCMLA_zpzzz(DisasContext *s, 
> arg_FCMLA_zpzzz *a)
>      }
>      if (sve_access_check(s)) {
>          unsigned vsz = vec_full_reg_size(s);
> -        unsigned desc;
> -        TCGv_i32 t_desc;
> -        TCGv_ptr pg = tcg_temp_new_ptr();
> -
> -        /* We would need 7 operands to pass these arguments "properly".
> -         * So we encode all the register numbers into the descriptor.
> -         */
> -        desc = deposit32(a->rd, 5, 5, a->rn);
> -        desc = deposit32(desc, 10, 5, a->rm);
> -        desc = deposit32(desc, 15, 5, a->ra);
> -        desc = deposit32(desc, 20, 2, a->rot);
> -        desc = sextract32(desc, 0, 22);
> -        desc = simd_desc(vsz, vsz, desc);
> -
> -        t_desc = tcg_const_i32(desc);
> -        tcg_gen_addi_ptr(pg, cpu_env, pred_full_reg_offset(s, a->pg));
> -        fns[a->esz - 1](cpu_env, pg, t_desc);
> -        tcg_temp_free_i32(t_desc);
> -        tcg_temp_free_ptr(pg);
> +        TCGv_ptr status = get_fpstatus_ptr(a->esz == MO_16);
> +        tcg_gen_gvec_5_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),
> +                           pred_full_reg_offset(s, a->pg),
> +                           status, vsz, vsz, a->rot, fns[a->esz]);
> +        tcg_temp_free_ptr(status);
>      }
>      return true;
>  }

Good to see the code simplified ;-)

Reviewed-by: Alex Bennée <address@hidden>

-- 
Alex Bennée