diff mbox series

[v2,09/17] target/arm: Replace ARM_FEATURE_VFP4 with isar_feature_aa32_simdfmac

Message ID 20200224222232.13807-10-richard.henderson@linaro.org
State New
Headers show
Series target/arm: vfp feature and decodetree cleanup | expand

Commit Message

Richard Henderson Feb. 24, 2020, 10:22 p.m. UTC 
All remaining tests for VFP4 are for fused multiply-add insns.

Since the MVFR1 field is used for both VFP and NEON, move its adjustment
from the !has_neon block to the (!has_vfp && !has_neon) block.

Test for vfp of the appropraite width alongside the test for simdfmac
within translate-vfp.inc.c.  Within disas_neon_data_insn, we have
already tested for ARM_FEATURE_NEON.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v2: Applied Peter's fixups, plus test fpdp_v2 in trans_VFM_dp.
---
 target/arm/cpu.h               | 12 ++++++++++++
 target/arm/cpu.c               |  6 +++++-
 target/arm/translate-vfp.inc.c | 22 ++++++++++++++++++----
 target/arm/translate.c         |  2 +-
 4 files changed, 36 insertions(+), 6 deletions(-)

-- 
2.20.1

Comments

Peter Maydell Feb. 25, 2020, 1:25 p.m. UTC | #1 
On Mon, 24 Feb 2020 at 22:22, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> All remaining tests for VFP4 are for fused multiply-add insns.

>

> Since the MVFR1 field is used for both VFP and NEON, move its adjustment

> from the !has_neon block to the (!has_vfp && !has_neon) block.

>

> Test for vfp of the appropraite width alongside the test for simdfmac


"appropriate"

> within translate-vfp.inc.c.  Within disas_neon_data_insn, we have

> already tested for ARM_FEATURE_NEON.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


Reviewed-by: Peter Maydell <peter.maydell@linaro.org>


thanks
-- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index b94d2a5ace..b29b0eddfc 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -3514,6 +3514,18 @@  static inline bool isar_feature_aa32_fp16_dpconv(const ARMISARegisters *id)
     return FIELD_EX32(id->mvfr1, MVFR1, FPHP) > 1;
 }
 
+/*
+ * Note that this ID register field covers both VFP and Neon FMAC,
+ * so should usually be tested in combination with some other
+ * check that confirms the presence of whichever of VFP or Neon is
+ * relevant, to avoid accidentally enabling a Neon feature on
+ * a VFP-no-Neon core or vice-versa.
+ */
+static inline bool isar_feature_aa32_simdfmac(const ARMISARegisters *id)
+{
+    return FIELD_EX32(id->mvfr1, MVFR1, SIMDFMAC) != 0;
+}
+
 static inline bool isar_feature_aa32_vsel(const ARMISARegisters *id)
 {
     return FIELD_EX32(id->mvfr2, MVFR2, FPMISC) >= 1;
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index f10f34b655..bdcaa46b8a 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1501,7 +1501,6 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         u = FIELD_DP32(u, MVFR1, SIMDINT, 0);
         u = FIELD_DP32(u, MVFR1, SIMDSP, 0);
         u = FIELD_DP32(u, MVFR1, SIMDHP, 0);
-        u = FIELD_DP32(u, MVFR1, SIMDFMAC, 0);
         cpu->isar.mvfr1 = u;
 
         u = cpu->isar.mvfr2;
@@ -1524,6 +1523,11 @@  static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
         u = cpu->isar.mvfr0;
         u = FIELD_DP32(u, MVFR0, SIMDREG, 0);
         cpu->isar.mvfr0 = u;
+
+        /* Despite the name, this field covers both VFP and Neon */
+        u = cpu->isar.mvfr1;
+        u = FIELD_DP32(u, MVFR1, SIMDFMAC, 0);
+        cpu->isar.mvfr1 = u;
     }
 
     if (arm_feature(env, ARM_FEATURE_AARCH64) &&
diff --git a/target/arm/translate-vfp.inc.c b/target/arm/translate-vfp.inc.c
index f88a95438f..03ba8d7aac 100644
--- a/target/arm/translate-vfp.inc.c
+++ b/target/arm/translate-vfp.inc.c
@@ -1803,11 +1803,18 @@  static bool trans_VFM_sp(DisasContext *s, arg_VFM_sp *a)
 
     /*
      * Present in VFPv4 only.
+     * Note that we can't rely on the SIMDFMAC check alone, because
+     * in a Neon-no-VFP core that ID register field will be non-zero.
+     */
+    if (!dc_isar_feature(aa32_simdfmac, s) ||
+        !dc_isar_feature(aa32_fpsp_v2, s)) {
+        return false;
+    }
+    /*
      * In v7A, UNPREDICTABLE with non-zero vector length/stride; from
      * v8A, must UNDEF. We choose to UNDEF for both v7A and v8A.
      */
-    if (!arm_dc_feature(s, ARM_FEATURE_VFP4) ||
-        (s->vec_len != 0 || s->vec_stride != 0)) {
+    if (s->vec_len != 0 || s->vec_stride != 0) {
         return false;
     }
 
@@ -1861,11 +1868,18 @@  static bool trans_VFM_dp(DisasContext *s, arg_VFM_dp *a)
 
     /*
      * Present in VFPv4 only.
+     * Note that we can't rely on the SIMDFMAC check alone, because
+     * in a Neon-no-VFP core that ID register field will be non-zero.
+     */
+    if (!dc_isar_feature(aa32_simdfmac, s) ||
+        !dc_isar_feature(aa32_fpdp_v2, s)) {
+        return false;
+    }
+    /*
      * In v7A, UNPREDICTABLE with non-zero vector length/stride; from
      * v8A, must UNDEF. We choose to UNDEF for both v7A and v8A.
      */
-    if (!arm_dc_feature(s, ARM_FEATURE_VFP4) ||
-        (s->vec_len != 0 || s->vec_stride != 0)) {
+    if (s->vec_len != 0 || s->vec_stride != 0) {
         return false;
     }
 
diff --git a/target/arm/translate.c b/target/arm/translate.c
index 79880adaad..0489e0cdaa 100644
--- a/target/arm/translate.c
+++ b/target/arm/translate.c
@@ -5150,7 +5150,7 @@  static int disas_neon_data_insn(DisasContext *s, uint32_t insn)
             }
             break;
         case NEON_3R_VFM_VQRDMLSH:
-            if (!arm_dc_feature(s, ARM_FEATURE_VFP4)) {
+            if (!dc_isar_feature(aa32_simdfmac, s)) {
                 return 1;
             }
             break;