Re: [Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them
On 6/14/19 10:52 AM, Peter Maydell wrote: > On Fri, 14 Jun 2019 at 18:21, Richard Henderson > wrote: >> >> On 6/14/19 3:44 AM, Peter Maydell wrote: >>> @@ -173,6 +173,11 @@ static bool trans_VSEL(DisasContext *s, arg_VSEL *a) >>> ((a->vm | a->vn | a->vd) & 0x10)) { >>> return false; >>> } >>> + >>> +if (dp && !dc_isar_feature(aa32_fpdp, s)) { >>> +return false; >>> +} >> >> Would it be cleaner to define something like >> >> static bool vfp_dp_enabled(DisasContext *s, int regmask) >> { >> if (!dc_isar_feature(aa32_fpdp, s)) { >> /* All double-precision disabled. */ >> return false; >> } >> if (!dc_isar_feature(aa32_fp_d32, s) && (regmask & 0x10)) { >> /* D16-D31 do not exist. */ >> return false; >> } >> return true; >> } >> >> Then use >> >> if (dp && !vfp_dp_enabled(s, a->vm | a->vn | a->vd)) >> >> ? > > It would be less code, but I don't think the "are we using > a register than doesn't exist" and the "do we have dp support" > checks are really related, and splitting the "OR the register > numbers together" from the "test the top bit" makes that > part look rather less clear I think. Fair enough. Reviewed-by: Richard Henderson r~
Re: [Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them
On Fri, 14 Jun 2019 at 18:21, Richard Henderson wrote: > > On 6/14/19 3:44 AM, Peter Maydell wrote: > > @@ -173,6 +173,11 @@ static bool trans_VSEL(DisasContext *s, arg_VSEL *a) > > ((a->vm | a->vn | a->vd) & 0x10)) { > > return false; > > } > > + > > +if (dp && !dc_isar_feature(aa32_fpdp, s)) { > > +return false; > > +} > > Would it be cleaner to define something like > > static bool vfp_dp_enabled(DisasContext *s, int regmask) > { > if (!dc_isar_feature(aa32_fpdp, s)) { > /* All double-precision disabled. */ > return false; > } > if (!dc_isar_feature(aa32_fp_d32, s) && (regmask & 0x10)) { > /* D16-D31 do not exist. */ > return false; > } > return true; > } > > Then use > > if (dp && !vfp_dp_enabled(s, a->vm | a->vn | a->vd)) > > ? It would be less code, but I don't think the "are we using a register than doesn't exist" and the "do we have dp support" checks are really related, and splitting the "OR the register numbers together" from the "test the top bit" makes that part look rather less clear I think. thanks -- PMM
Re: [Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them
On 6/14/19 3:44 AM, Peter Maydell wrote: > @@ -173,6 +173,11 @@ static bool trans_VSEL(DisasContext *s, arg_VSEL *a) > ((a->vm | a->vn | a->vd) & 0x10)) { > return false; > } > + > +if (dp && !dc_isar_feature(aa32_fpdp, s)) { > +return false; > +} Would it be cleaner to define something like static bool vfp_dp_enabled(DisasContext *s, int regmask) { if (!dc_isar_feature(aa32_fpdp, s)) { /* All double-precision disabled. */ return false; } if (!dc_isar_feature(aa32_fp_d32, s) && (regmask & 0x10)) { /* D16-D31 do not exist. */ return false; } return true; } Then use if (dp && !vfp_dp_enabled(s, a->vm | a->vn | a->vd)) ? r~