[Qemu-devel] [PATCH 0/2] target/arm: Support single-precision only FPUs

[Qemu-devel] [PATCH 0/2] target/arm: Support single-precision only FPUs

[Peter Maydell]

The Arm architecture permits FPUs which have only single-precision
support, not double-precision; Cortex-M4 and Cortex-M33 are
both like that. Now that we've refactored the VFP code to use
decodetree it's fairly easy to add the necessary checks on the
MVFR0 FPDP field so that we UNDEF any double-precision instructions
on CPUs like this.

The first patch fixes some no-visible-effect typos in the
names of struct arguments to some functions (caused by
cut-n-paste errors); not really related but I noticed them
while I was working on this.

thanks
-- PMM

Peter Maydell (2):
  target/arm: Fix typos in trans function prototypes
  target/arm: Only implement doubles if the FPU supports them

 target/arm/cpu.h               |   6 ++
 target/arm/translate-vfp.inc.c | 112 ++++++++++++++++++++++++++++-----
 2 files changed, 104 insertions(+), 14 deletions(-)

-- 
2.20.1

[Qemu-devel] [PATCH 1/2] target/arm: Fix typos in trans function prototypes

[Peter Maydell]

In several places cut and paste errors meant we were using the wrong
type for the 'arg' struct in trans_ functions called by the
decodetree decoder, because we were using the _sp version of the
struct in the _dp function.  These were harmless, because the two
structs were identical and so decodetree made them typedefs of the
same underlying structure (and we'd have had a compile error if they
were not harmless), but we should clean them up anyway.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/translate-vfp.inc.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 709fc65374d..85187bcc9dc 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -835,7 +835,7 @@ static bool trans_VMOV_64_sp(DisasContext *s, arg_VMOV_64_sp *a)
     return true;
 }
 
-static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_sp *a)
+static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_dp *a)
 {
     TCGv_i32 tmp;
 
@@ -910,7 +910,7 @@ static bool trans_VLDR_VSTR_sp(DisasContext *s, arg_VLDR_VSTR_sp *a)
     return true;
 }
 
-static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_sp *a)
+static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_dp *a)
 {
     uint32_t offset;
     TCGv_i32 addr;
@@ -1500,7 +1500,7 @@ static void gen_VMLA_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
     tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_sp *a)
+static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_dp *a)
 {
     return do_vfp_3op_dp(s, gen_VMLA_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1538,7 +1538,7 @@ static void gen_VMLS_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
     tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_sp *a)
+static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_dp *a)
 {
     return do_vfp_3op_dp(s, gen_VMLS_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1580,7 +1580,7 @@ static void gen_VNMLS_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
     tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_sp *a)
+static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_dp *a)
 {
     return do_vfp_3op_dp(s, gen_VNMLS_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1614,7 +1614,7 @@ static void gen_VNMLA_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
     tcg_temp_free_i64(tmp);
 }
 
-static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_sp *a)
+static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_dp *a)
 {
     return do_vfp_3op_dp(s, gen_VNMLA_dp, a->vd, a->vn, a->vm, true);
 }
@@ -1624,7 +1624,7 @@ static bool trans_VMUL_sp(DisasContext *s, arg_VMUL_sp *a)
     return do_vfp_3op_sp(s, gen_helper_vfp_muls, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_sp *a)
+static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_dp *a)
 {
     return do_vfp_3op_dp(s, gen_helper_vfp_muld, a->vd, a->vn, a->vm, false);
 }
@@ -1648,7 +1648,7 @@ static void gen_VNMUL_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
     gen_helper_vfp_negd(vd, vd);
 }
 
-static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_sp *a)
+static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_dp *a)
 {
     return do_vfp_3op_dp(s, gen_VNMUL_dp, a->vd, a->vn, a->vm, false);
 }
@@ -1658,7 +1658,7 @@ static bool trans_VADD_sp(DisasContext *s, arg_VADD_sp *a)
     return do_vfp_3op_sp(s, gen_helper_vfp_adds, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VADD_dp(DisasContext *s, arg_VADD_sp *a)
+static bool trans_VADD_dp(DisasContext *s, arg_VADD_dp *a)
 {
     return do_vfp_3op_dp(s, gen_helper_vfp_addd, a->vd, a->vn, a->vm, false);
 }
@@ -1668,7 +1668,7 @@ static bool trans_VSUB_sp(DisasContext *s, arg_VSUB_sp *a)
     return do_vfp_3op_sp(s, gen_helper_vfp_subs, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_sp *a)
+static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_dp *a)
 {
     return do_vfp_3op_dp(s, gen_helper_vfp_subd, a->vd, a->vn, a->vm, false);
 }
@@ -1678,7 +1678,7 @@ static bool trans_VDIV_sp(DisasContext *s, arg_VDIV_sp *a)
     return do_vfp_3op_sp(s, gen_helper_vfp_divs, a->vd, a->vn, a->vm, false);
 }
 
-static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_sp *a)
+static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_dp *a)
 {
     return do_vfp_3op_dp(s, gen_helper_vfp_divd, a->vd, a->vn, a->vm, false);
 }
@@ -1741,7 +1741,7 @@ static bool trans_VFM_sp(DisasContext *s, arg_VFM_sp *a)
     return true;
 }
 
-static bool trans_VFM_dp(DisasContext *s, arg_VFM_sp *a)
+static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
 {
     /*
      * VFNMA : fd = muladd(-fd,  fn, fm)
@@ -2201,7 +2201,7 @@ static bool trans_VRINTR_sp(DisasContext *s, arg_VRINTR_sp *a)
     return true;
 }
 
-static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_sp *a)
+static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_dp *a)
 {
     TCGv_ptr fpst;
     TCGv_i64 tmp;
@@ -2257,7 +2257,7 @@ static bool trans_VRINTZ_sp(DisasContext *s, arg_VRINTZ_sp *a)
     return true;
 }
 
-static bool trans_VRINTZ_dp(DisasContext *s, arg_VRINTZ_sp *a)
+static bool trans_VRINTZ_dp(DisasContext *s, arg_VRINTZ_dp *a)
 {
     TCGv_ptr fpst;
     TCGv_i64 tmp;
-- 
2.20.1

Re: [Qemu-devel] [PATCH 1/2] target/arm: Fix typos in trans function prototypes

[Philippe Mathieu-Daudé]

On 6/14/19 12:44 PM, Peter Maydell wrote:
> In several places cut and paste errors meant we were using the wrong
> type for the 'arg' struct in trans_ functions called by the
> decodetree decoder, because we were using the _sp version of the
> struct in the _dp function.  These were harmless, because the two
> structs were identical and so decodetree made them typedefs of the
> same underlying structure (and we'd have had a compile error if they
> were not harmless), but we should clean them up anyway.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/translate-vfp.inc.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
> index 709fc65374d..85187bcc9dc 100644
> --- a/target/arm/translate-vfp.inc.c
> +++ b/target/arm/translate-vfp.inc.c
> @@ -835,7 +835,7 @@ static bool trans_VMOV_64_sp(DisasContext *s, arg_VMOV_64_sp *a)
>      return true;
>  }
>  
> -static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_sp *a)
> +static bool trans_VMOV_64_dp(DisasContext *s, arg_VMOV_64_dp *a)
>  {
>      TCGv_i32 tmp;
>  
> @@ -910,7 +910,7 @@ static bool trans_VLDR_VSTR_sp(DisasContext *s, arg_VLDR_VSTR_sp *a)
>      return true;
>  }
>  
> -static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_sp *a)
> +static bool trans_VLDR_VSTR_dp(DisasContext *s, arg_VLDR_VSTR_dp *a)
>  {
>      uint32_t offset;
>      TCGv_i32 addr;
> @@ -1500,7 +1500,7 @@ static void gen_VMLA_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
>      tcg_temp_free_i64(tmp);
>  }
>  
> -static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_sp *a)
> +static bool trans_VMLA_dp(DisasContext *s, arg_VMLA_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_VMLA_dp, a->vd, a->vn, a->vm, true);
>  }
> @@ -1538,7 +1538,7 @@ static void gen_VMLS_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
>      tcg_temp_free_i64(tmp);
>  }
>  
> -static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_sp *a)
> +static bool trans_VMLS_dp(DisasContext *s, arg_VMLS_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_VMLS_dp, a->vd, a->vn, a->vm, true);
>  }
> @@ -1580,7 +1580,7 @@ static void gen_VNMLS_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
>      tcg_temp_free_i64(tmp);
>  }
>  
> -static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_sp *a)
> +static bool trans_VNMLS_dp(DisasContext *s, arg_VNMLS_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_VNMLS_dp, a->vd, a->vn, a->vm, true);
>  }
> @@ -1614,7 +1614,7 @@ static void gen_VNMLA_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
>      tcg_temp_free_i64(tmp);
>  }
>  
> -static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_sp *a)
> +static bool trans_VNMLA_dp(DisasContext *s, arg_VNMLA_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_VNMLA_dp, a->vd, a->vn, a->vm, true);
>  }
> @@ -1624,7 +1624,7 @@ static bool trans_VMUL_sp(DisasContext *s, arg_VMUL_sp *a)
>      return do_vfp_3op_sp(s, gen_helper_vfp_muls, a->vd, a->vn, a->vm, false);
>  }
>  
> -static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_sp *a)
> +static bool trans_VMUL_dp(DisasContext *s, arg_VMUL_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_helper_vfp_muld, a->vd, a->vn, a->vm, false);
>  }
> @@ -1648,7 +1648,7 @@ static void gen_VNMUL_dp(TCGv_i64 vd, TCGv_i64 vn, TCGv_i64 vm, TCGv_ptr fpst)
>      gen_helper_vfp_negd(vd, vd);
>  }
>  
> -static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_sp *a)
> +static bool trans_VNMUL_dp(DisasContext *s, arg_VNMUL_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_VNMUL_dp, a->vd, a->vn, a->vm, false);
>  }
> @@ -1658,7 +1658,7 @@ static bool trans_VADD_sp(DisasContext *s, arg_VADD_sp *a)
>      return do_vfp_3op_sp(s, gen_helper_vfp_adds, a->vd, a->vn, a->vm, false);
>  }
>  
> -static bool trans_VADD_dp(DisasContext *s, arg_VADD_sp *a)
> +static bool trans_VADD_dp(DisasContext *s, arg_VADD_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_helper_vfp_addd, a->vd, a->vn, a->vm, false);
>  }
> @@ -1668,7 +1668,7 @@ static bool trans_VSUB_sp(DisasContext *s, arg_VSUB_sp *a)
>      return do_vfp_3op_sp(s, gen_helper_vfp_subs, a->vd, a->vn, a->vm, false);
>  }
>  
> -static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_sp *a)
> +static bool trans_VSUB_dp(DisasContext *s, arg_VSUB_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_helper_vfp_subd, a->vd, a->vn, a->vm, false);
>  }
> @@ -1678,7 +1678,7 @@ static bool trans_VDIV_sp(DisasContext *s, arg_VDIV_sp *a)
>      return do_vfp_3op_sp(s, gen_helper_vfp_divs, a->vd, a->vn, a->vm, false);
>  }
>  
> -static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_sp *a)
> +static bool trans_VDIV_dp(DisasContext *s, arg_VDIV_dp *a)
>  {
>      return do_vfp_3op_dp(s, gen_helper_vfp_divd, a->vd, a->vn, a->vm, false);
>  }
> @@ -1741,7 +1741,7 @@ static bool trans_VFM_sp(DisasContext *s, arg_VFM_sp *a)
>      return true;
>  }
>  
> -static bool trans_VFM_dp(DisasContext *s, arg_VFM_sp *a)
> +static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
>  {
>      /*
>       * VFNMA : fd = muladd(-fd,  fn, fm)
> @@ -2201,7 +2201,7 @@ static bool trans_VRINTR_sp(DisasContext *s, arg_VRINTR_sp *a)
>      return true;
>  }
>  
> -static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_sp *a)
> +static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_dp *a)
>  {
>      TCGv_ptr fpst;
>      TCGv_i64 tmp;
> @@ -2257,7 +2257,7 @@ static bool trans_VRINTZ_sp(DisasContext *s, arg_VRINTZ_sp *a)
>      return true;
>  }
>  
> -static bool trans_VRINTZ_dp(DisasContext *s, arg_VRINTZ_sp *a)
> +static bool trans_VRINTZ_dp(DisasContext *s, arg_VRINTZ_dp *a)
>  {
>      TCGv_ptr fpst;
>      TCGv_i64 tmp;
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

[Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them

[Peter Maydell]

The architecture permits FPUs which have only single-precision
support, not double-precision; Cortex-M4 and Cortex-M33 are
both like that. Add the necessary checks on the MVFR0 FPDP
field so that we UNDEF any double-precision instructions on
CPUs like this.

Note that even if FPDP==0 the insns like VMOV-to/from-gpreg,
VLDM/VSTM, VLDR/VSTR which take double precision registers
still exist.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h               |  6 +++
 target/arm/translate-vfp.inc.c | 84 ++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 92298624215..29be1f7ea97 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3382,6 +3382,12 @@ static inline bool isar_feature_aa32_fpshvec(const ARMISARegisters *id)
     return FIELD_EX64(id->mvfr0, MVFR0, FPSHVEC) > 0;
 }
 
+static inline bool isar_feature_aa32_fpdp(const ARMISARegisters *id)
+{
+    /* Return true if CPU supports double precision floating point */
+    return FIELD_EX64(id->mvfr0, MVFR0, FPDP) > 0;
+}
+
 /*
  * We always set the FP and SIMD FP16 fields to indicate identical
  * levels of support (assuming SIMD is implemented at all), so
diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index 85187bcc9dc..a3df81d3b07 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -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;
+    }
+
     rd = a->vd;
     rn = a->vn;
     rm = a->vm;
@@ -301,6 +306,11 @@ static bool trans_VMINMAXNM(DisasContext *s, arg_VMINMAXNM *a)
         ((a->vm | a->vn | a->vd) & 0x10)) {
         return false;
     }
+
+    if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     rd = a->vd;
     rn = a->vn;
     rm = a->vm;
@@ -382,6 +392,11 @@ static bool trans_VRINT(DisasContext *s, arg_VRINT *a)
         ((a->vm | a->vd) & 0x10)) {
         return false;
     }
+
+    if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     rd = a->vd;
     rm = a->vm;
 
@@ -440,6 +455,11 @@ static bool trans_VCVT(DisasContext *s, arg_VCVT *a)
     if (dp && !dc_isar_feature(aa32_fp_d32, s) && (a->vm & 0x10)) {
         return false;
     }
+
+    if (dp && !dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     rd = a->vd;
     rm = a->vm;
 
@@ -1268,6 +1288,10 @@ static bool do_vfp_3op_dp(DisasContext *s, VFPGen3OpDPFn *fn,
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!dc_isar_feature(aa32_fpshvec, s) &&
         (veclen != 0 || s->vec_stride != 0)) {
         return false;
@@ -1413,6 +1437,10 @@ static bool do_vfp_2op_dp(DisasContext *s, VFPGen2OpDPFn *fn, int vd, int vm)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!dc_isar_feature(aa32_fpshvec, s) &&
         (veclen != 0 || s->vec_stride != 0)) {
         return false;
@@ -1773,6 +1801,10 @@ static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -1878,6 +1910,10 @@ static bool trans_VMOV_imm_dp(DisasContext *s, arg_VMOV_imm_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!dc_isar_feature(aa32_fpshvec, s) &&
         (veclen != 0 || s->vec_stride != 0)) {
         return false;
@@ -2028,6 +2064,10 @@ static bool trans_VCMP_dp(DisasContext *s, arg_VCMP_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2097,6 +2137,10 @@ static bool trans_VCVT_f64_f16(DisasContext *s, arg_VCVT_f64_f16 *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2159,6 +2203,10 @@ static bool trans_VCVT_f16_f64(DisasContext *s, arg_VCVT_f16_f64 *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2215,6 +2263,10 @@ static bool trans_VRINTR_dp(DisasContext *s, arg_VRINTR_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2272,6 +2324,10 @@ static bool trans_VRINTZ_dp(DisasContext *s, arg_VRINTZ_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2327,6 +2383,10 @@ static bool trans_VRINTX_dp(DisasContext *s, arg_VRINTX_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2351,6 +2411,10 @@ static bool trans_VCVT_sp(DisasContext *s, arg_VCVT_sp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2375,6 +2439,10 @@ static bool trans_VCVT_dp(DisasContext *s, arg_VCVT_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2425,6 +2493,10 @@ static bool trans_VCVT_int_dp(DisasContext *s, arg_VCVT_int_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2461,6 +2533,10 @@ static bool trans_VJCVT(DisasContext *s, arg_VJCVT *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2550,6 +2626,10 @@ static bool trans_VCVT_fix_dp(DisasContext *s, arg_VCVT_fix_dp *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
@@ -2642,6 +2722,10 @@ static bool trans_VCVT_dp_int(DisasContext *s, arg_VCVT_dp_int *a)
         return false;
     }
 
+    if (!dc_isar_feature(aa32_fpdp, s)) {
+        return false;
+    }
+
     if (!vfp_access_check(s)) {
         return true;
     }
-- 
2.20.1

Re: [Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them

[Richard Henderson]

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~

Re: [Qemu-devel] [PATCH 2/2] target/arm: Only implement doubles if the FPU supports them

[Peter Maydell]

On Fri, 14 Jun 2019 at 18:21, Richard Henderson
<richard.henderson@linaro.org> 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

[Richard Henderson]

On 6/14/19 10:52 AM, Peter Maydell wrote:
> On Fri, 14 Jun 2019 at 18:21, Richard Henderson
> <richard.henderson@linaro.org> 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 <richard.henderson@linaro.org>


r~