diff mbox series

[2/2] target/arm: Only implement doubles if the FPU supports them

Message ID 20190614104457.24703-3-peter.maydell@linaro.org
State Superseded
Headers show
Series target/arm: Support single-precision only FPUs | expand

Commit Message

Peter Maydell June 14, 2019, 10:44 a.m. UTC 
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(+)

-- 
2.20.1

Comments

Richard Henderson June 14, 2019, 5:21 p.m. UTC | #1 
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~
Peter Maydell June 14, 2019, 5:52 p.m. UTC | #2 
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
Richard Henderson June 14, 2019, 9:45 p.m. UTC | #3 
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~
diff mbox series

Patch

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;
     }