Re: [PATCH v1 04/11] target/arm: Implement vector float32 to bfloat16 conversion

[Richard Henderson]

On 5/18/21 6:10 AM, Peter Maydell wrote:
> On Sat, 17 Apr 2021 at 01:03, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> > This is BFCVT{N,T} for both AArch64 AdvSIMD and SVE,
> > and VCVT.BF16.F32 for AArch32 NEON.
> >
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>
> > @@ -7567,6 +7568,7 @@ void HELPER(NAME)(void *vd, void *vn, void *vg, void 
> > *status, uint32_t desc)  \
> >   }
> >
> >   DO_FCVTNT(sve2_fcvtnt_sh, uint32_t, uint16_t, H1_4, H1_2, sve_f32_to_f16)
> > +DO_FCVTNT(sve_bfcvtnt,    uint32_t, uint16_t, H1_4, H1_2, float32_to_bfloat16)
> >   DO_FCVTNT(sve2_fcvtnt_ds, uint64_t, uint32_t, H1_4, H1_2, float64_to_float32)
>
> Not related to this patch, but are the H macros for sve2_fcvtnt_ds definitely
> right? Just noticed they're the same as the ones being used for the f32->f16
> helpers despite the types being different sizes.

Definitely wrong, and now fixed in the sve2 patch set (need to fix some 
regressions there before re-posting).

> > +    if (!dc_isar_feature(aa32_bf16, s)) {
> > +        return false;
> > +    }
>
> Do we need to also check ARM_FEATURE_NEON here ?

Hmm, I dunno.  Since FEAT_AA32BF16 has both VFP and NEON instructions, I guess 
we could turn off one without the other.

I'll add it.


r~