[Qemu-devel] [PATCH v7 0/6] target-arm: Add support for VSEL and VMIN/MAXNM.

[Qemu-devel] [PATCH v7 0/6] target-arm: Add support for VSEL and VMIN/MAXNM.

[Will Newton]

This series adds support for three new instructions added in ARMv8 -
VSEL, VMINNM and VMAXNM.

Will Newton (6):
  target-arm: Move call to disas_vfp_insn out of disas_coproc_insn.
  target-arm: Implement ARMv8 VSEL instruction.
  softfloat: Remove unused argument from MINMAX macro.
  softfloat: Add minNum() and maxNum() functions to softfloat.
  target-arm: Implement ARMv8 FP VMAXNM and VMINNM instructions.
  target-arm: Implement ARMv8 SIMD VMAXNM and VMINNM instructions.

 fpu/softfloat.c          |  60 +++++++++++-
 include/fpu/softfloat.h  |   4 +
 target-arm/helper.c      |  25 +++++
 target-arm/helper.h      |   8 ++
 target-arm/neon_helper.c |  16 ++++
 target-arm/translate.c   | 244 ++++++++++++++++++++++++++++++++++++++++++++---
 6 files changed, 340 insertions(+), 17 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH v7 1/6] target-arm: Move call to disas_vfp_insn out of disas_coproc_insn.

[Will Newton]

Floating point is an extension to the instruction set rather than
a coprocessor, so call it directly from the ARM and Thumb decode
functions.
---
 target-arm/translate.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

Changes in v7:
 - Fix comment style
 - Fix brace style

diff --git a/target-arm/translate.c b/target-arm/translate.c
index 5f003e7..f63e89d 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2636,6 +2636,14 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
             && rn != ARM_VFP_MVFR1 && rn != ARM_VFP_MVFR0)
             return 1;
     }
+
+    if (extract32(insn, 28, 4) == 0xf) {
+        /* Encodings with T=1 (Thumb) or unconditional (ARM):
+         * only used in v8 and above.
+         */
+        return 1;
+    }
+
     dp = ((insn & 0xf00) == 0xb00);
     switch ((insn >> 24) & 0xf) {
     case 0xe:
@@ -6296,9 +6304,6 @@ static int disas_coproc_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
 	    return disas_dsp_insn(env, s, insn);
 	}
 	return 1;
-    case 10:
-    case 11:
-	return disas_vfp_insn (env, s, insn);
     default:
         break;
     }
@@ -6753,6 +6758,13 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
                 goto illegal_op;
             return;
         }
+        if ((insn & 0x0f000e10) == 0x0e000a00) {
+            /* VFP.  */
+            if (disas_vfp_insn(env, s, insn)) {
+                goto illegal_op;
+            }
+            return;
+        }
         if (((insn & 0x0f30f000) == 0x0510f000) ||
             ((insn & 0x0f30f010) == 0x0710f000)) {
             if ((insn & (1 << 22)) == 0) {
@@ -8033,9 +8045,15 @@ static void disas_arm_insn(CPUARMState * env, DisasContext *s)
         case 0xc:
         case 0xd:
         case 0xe:
-            /* Coprocessor.  */
-            if (disas_coproc_insn(env, s, insn))
+            if (((insn >> 8) & 0xe) == 10) {
+                /* VFP.  */
+                if (disas_vfp_insn(env, s, insn)) {
+                    goto illegal_op;
+                }
+            } else if (disas_coproc_insn(env, s, insn)) {
+                /* Coprocessor.  */
                 goto illegal_op;
+            }
             break;
         case 0xf:
             /* swi */
@@ -8765,6 +8783,10 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
             insn = (insn & 0xe2ffffff) | ((insn & (1 << 28)) >> 4) | (1 << 28);
             if (disas_neon_data_insn(env, s, insn))
                 goto illegal_op;
+        } else if (((insn >> 8) & 0xe) == 10) {
+            if (disas_vfp_insn(env, s, insn)) {
+                goto illegal_op;
+            }
         } else {
             if (insn & (1 << 28))
                 goto illegal_op;
-- 
1.8.1.4

[Qemu-devel] [PATCH v7 2/6] target-arm: Implement ARMv8 VSEL instruction.

[Will Newton]

This adds support for the VSEL floating point selection instruction
which was added in ARMv8.
---
 target-arm/translate.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 134 insertions(+), 1 deletion(-)

Changes in v7:
 - Break out VSEL handling into a function
 - Properly sign extend VF and NF to 64bit
 - Use extract32 to decode insn
 - Fix brace style

diff --git a/target-arm/translate.c b/target-arm/translate.c
index f63e89d..0a22ad8 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2614,6 +2614,139 @@ static TCGv_i32 gen_load_and_replicate(DisasContext *s, TCGv_i32 addr, int size)
     return tmp;
 }
 
+static int handle_vsel(uint32_t insn, uint32_t rd, uint32_t rn, uint32_t rm,
+                       uint32_t dp)
+{
+    uint32_t cc = extract32(insn, 20, 2);
+
+    if (dp) {
+        TCGv_i64 frn, frm, dest;
+        TCGv_i64 tmp, zero, zf, nf, vf;
+
+        zero = tcg_const_i64(0);
+
+        frn = tcg_temp_new_i64();
+        frm = tcg_temp_new_i64();
+        dest = tcg_temp_new_i64();
+
+        zf = tcg_temp_new_i64();
+        nf = tcg_temp_new_i64();
+        vf = tcg_temp_new_i64();
+
+        tcg_gen_extu_i32_i64(zf, cpu_ZF);
+        tcg_gen_ext_i32_i64(nf, cpu_NF);
+        tcg_gen_ext_i32_i64(vf, cpu_VF);
+
+        tcg_gen_ld_f64(frn, cpu_env, vfp_reg_offset(dp, rn));
+        tcg_gen_ld_f64(frm, cpu_env, vfp_reg_offset(dp, rm));
+        switch (cc) {
+        case 0: /* eq: Z */
+            tcg_gen_movcond_i64(TCG_COND_EQ, dest, zf, zero,
+                                frn, frm);
+            break;
+        case 1: /* vs: V */
+            tcg_gen_movcond_i64(TCG_COND_LT, dest, vf, zero,
+                                frn, frm);
+            break;
+        case 2: /* ge: N == V -> N ^ V == 0 */
+            tmp = tcg_temp_new_i64();
+            tcg_gen_xor_i64(tmp, vf, nf);
+            tcg_gen_movcond_i64(TCG_COND_GE, dest, tmp, zero,
+                                frn, frm);
+            tcg_temp_free_i64(tmp);
+            break;
+        case 3: /* gt: !Z && N == V */
+            tcg_gen_movcond_i64(TCG_COND_NE, dest, zf, zero,
+                                frn, frm);
+            tmp = tcg_temp_new_i64();
+            tcg_gen_xor_i64(tmp, vf, nf);
+            tcg_gen_movcond_i64(TCG_COND_GE, dest, tmp, zero,
+                                dest, frm);
+            tcg_temp_free_i64(tmp);
+            break;
+        }
+        tcg_gen_st_f64(dest, cpu_env, vfp_reg_offset(dp, rd));
+        tcg_temp_free_i64(frn);
+        tcg_temp_free_i64(frm);
+        tcg_temp_free_i64(dest);
+
+        tcg_temp_free_i64(zf);
+        tcg_temp_free_i64(nf);
+        tcg_temp_free_i64(vf);
+
+        tcg_temp_free_i64(zero);
+    } else {
+        TCGv_i32 frn, frm, dest;
+        TCGv_i32 tmp, zero;
+
+        zero = tcg_const_i32(0);
+
+        frn = tcg_temp_new_i32();
+        frm = tcg_temp_new_i32();
+        dest = tcg_temp_new_i32();
+        tcg_gen_ld_f32(frn, cpu_env, vfp_reg_offset(dp, rn));
+        tcg_gen_ld_f32(frm, cpu_env, vfp_reg_offset(dp, rm));
+        switch (cc) {
+        case 0: /* eq: Z */
+            tcg_gen_movcond_i32(TCG_COND_EQ, dest, cpu_ZF, zero,
+                                frn, frm);
+            break;
+        case 1: /* vs: V */
+            tcg_gen_movcond_i32(TCG_COND_LT, dest, cpu_VF, zero,
+                                frn, frm);
+            break;
+        case 2: /* ge: N == V -> N ^ V == 0 */
+            tmp = tcg_temp_new_i32();
+            tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
+            tcg_gen_movcond_i32(TCG_COND_GE, dest, tmp, zero,
+                                frn, frm);
+            tcg_temp_free_i32(tmp);
+            break;
+        case 3: /* gt: !Z && N == V */
+            tcg_gen_movcond_i32(TCG_COND_NE, dest, cpu_ZF, zero,
+                                frn, frm);
+            tmp = tcg_temp_new_i32();
+            tcg_gen_xor_i32(tmp, cpu_VF, cpu_NF);
+            tcg_gen_movcond_i32(TCG_COND_GE, dest, tmp, zero,
+                                dest, frm);
+            tcg_temp_free_i32(tmp);
+            break;
+        }
+        tcg_gen_st_f32(dest, cpu_env, vfp_reg_offset(dp, rd));
+        tcg_temp_free_i32(frn);
+        tcg_temp_free_i32(frm);
+        tcg_temp_free_i32(dest);
+
+        tcg_temp_free_i32(zero);
+    }
+
+    return 0;
+}
+
+static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
+{
+    uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
+
+    if (!arm_feature(env, ARM_FEATURE_V8)) {
+        return 1;
+    }
+
+    if (dp) {
+        VFP_DREG_D(rd, insn);
+        VFP_DREG_N(rn, insn);
+        VFP_DREG_M(rm, insn);
+    } else {
+        rd = VFP_SREG_D(insn);
+        rn = VFP_SREG_N(insn);
+        rm = VFP_SREG_M(insn);
+    }
+
+    if ((insn & 0x0f800e50) == 0x0e000a00) {
+        return handle_vsel(insn, rd, rn, rm, dp);
+    }
+    return 1;
+}
+
 /* Disassemble a VFP instruction.  Returns nonzero if an error occurred
    (ie. an undefined instruction).  */
 static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
@@ -2641,7 +2774,7 @@ static int disas_vfp_insn(CPUARMState * env, DisasContext *s, uint32_t insn)
         /* Encodings with T=1 (Thumb) or unconditional (ARM):
          * only used in v8 and above.
          */
-        return 1;
+        return disas_vfp_v8_insn(env, s, insn);
     }
 
     dp = ((insn & 0xf00) == 0xb00);
-- 
1.8.1.4

[Qemu-devel] [PATCH v7 3/6] softfloat: Remove unused argument from MINMAX macro.

[Will Newton]

The nan_exp argument is not used, so remove it.
---
 fpu/softfloat.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Changes in v7:
 - New patch

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 7ba51b6..97bf627 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6706,7 +6706,7 @@ int float128_compare_quiet( float128 a, float128 b STATUS_PARAM )
  * 'compare and pick one input' because that would mishandle
  * NaNs and +0 vs -0.
  */
-#define MINMAX(s, nan_exp)                                              \
+#define MINMAX(s)                                                       \
 INLINE float ## s float ## s ## _minmax(float ## s a, float ## s b,     \
                                         int ismin STATUS_PARAM )        \
 {                                                                       \
@@ -6747,8 +6747,8 @@ float ## s float ## s ## _max(float ## s a, float ## s b STATUS_PARAM)  \
     return float ## s ## _minmax(a, b, 0 STATUS_VAR);                   \
 }
 
-MINMAX(32, 0xff)
-MINMAX(64, 0x7ff)
+MINMAX(32)
+MINMAX(64)
 
 
 /* Multiply A by 2 raised to the power N.  */
-- 
1.8.1.4

[Qemu-devel] [PATCH v7 4/6] softfloat: Add minNum() and maxNum() functions to softfloat.

[Will Newton]

Add floatnn_minnum() and floatnn_maxnum() functions which are equivalent
to the minNum() and maxNum() functions from IEEE 754-2008. They are
similar to min() and max() but differ in the handling of QNaN arguments.
---
 fpu/softfloat.c         | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/fpu/softfloat.h |  4 ++++
 2 files changed, 58 insertions(+)

Changes in v7:
 - New patch

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 97bf627..9834927 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6750,6 +6750,60 @@ float ## s float ## s ## _max(float ## s a, float ## s b STATUS_PARAM)  \
 MINMAX(32)
 MINMAX(64)
 
+/* minnum() and maxnum() functions. These are similar to the min()
+ * and max() functions but if one of the arguments is a QNaN and
+ * the other is numerical then the numerical argument is returned.
+ */
+#define MINMAXNUM(s)                                                       \
+INLINE float ## s float ## s ## _minmaxnum(float ## s a, float ## s b,     \
+                                           int ismin STATUS_PARAM )        \
+{                                                                          \
+    flag aSign, bSign;                                                     \
+    uint ## s ## _t av, bv;                                                \
+    a = float ## s ## _squash_input_denormal(a STATUS_VAR);                \
+    b = float ## s ## _squash_input_denormal(b STATUS_VAR);                \
+    if (float ## s ## _is_quiet_nan(a) &&                                  \
+        !float ## s ##_is_quiet_nan(b)) {                                  \
+        return b;                                                          \
+    } else if (float ## s ## _is_quiet_nan(b) &&                           \
+               !float ## s ## _is_quiet_nan(a)) {                          \
+        return a;                                                          \
+    } else if (float ## s ## _is_any_nan(a) ||                             \
+        float ## s ## _is_any_nan(b)) {                                    \
+        return propagateFloat ## s ## NaN(a, b STATUS_VAR);                \
+    }                                                                      \
+    aSign = extractFloat ## s ## Sign(a);                                  \
+    bSign = extractFloat ## s ## Sign(b);                                  \
+    av = float ## s ## _val(a);                                            \
+    bv = float ## s ## _val(b);                                            \
+    if (aSign != bSign) {                                                  \
+        if (ismin) {                                                       \
+            return aSign ? a : b;                                          \
+        } else {                                                           \
+            return aSign ? b : a;                                          \
+        }                                                                  \
+    } else {                                                               \
+        if (ismin) {                                                       \
+            return (aSign ^ (av < bv)) ? a : b;                            \
+        } else {                                                           \
+            return (aSign ^ (av < bv)) ? b : a;                            \
+        }                                                                  \
+    }                                                                      \
+}                                                                          \
+                                                                           \
+float ## s float ## s ## _minnum(float ## s a, float ## s b STATUS_PARAM)  \
+{                                                                          \
+    return float ## s ## _minmaxnum(a, b, 1 STATUS_VAR);                   \
+}                                                                          \
+                                                                           \
+float ## s float ## s ## _maxnum(float ## s a, float ## s b STATUS_PARAM)  \
+{                                                                          \
+    return float ## s ## _minmaxnum(a, b, 0 STATUS_VAR);                   \
+}
+
+MINMAXNUM(32)
+MINMAXNUM(64)
+
 
 /* Multiply A by 2 raised to the power N.  */
 float32 float32_scalbn( float32 a, int n STATUS_PARAM )
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index f3927e2..2365274 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -302,6 +302,8 @@ int float32_compare( float32, float32 STATUS_PARAM );
 int float32_compare_quiet( float32, float32 STATUS_PARAM );
 float32 float32_min(float32, float32 STATUS_PARAM);
 float32 float32_max(float32, float32 STATUS_PARAM);
+float32 float32_minnum(float32, float32 STATUS_PARAM);
+float32 float32_maxnum(float32, float32 STATUS_PARAM);
 int float32_is_quiet_nan( float32 );
 int float32_is_signaling_nan( float32 );
 float32 float32_maybe_silence_nan( float32 );
@@ -408,6 +410,8 @@ int float64_compare( float64, float64 STATUS_PARAM );
 int float64_compare_quiet( float64, float64 STATUS_PARAM );
 float64 float64_min(float64, float64 STATUS_PARAM);
 float64 float64_max(float64, float64 STATUS_PARAM);
+float64 float64_minnum(float64, float64 STATUS_PARAM);
+float64 float64_maxnum(float64, float64 STATUS_PARAM);
 int float64_is_quiet_nan( float64 a );
 int float64_is_signaling_nan( float64 );
 float64 float64_maybe_silence_nan( float64 );
-- 
1.8.1.4

[Qemu-devel] [PATCH v7 5/6] target-arm: Implement ARMv8 FP VMAXNM and VMINNM instructions.

[Will Newton]

This adds support for the ARMv8 floating point VMAXNM and VMINNM
instructions.
---
 target-arm/helper.c    | 25 +++++++++++++++++++++++++
 target-arm/helper.h    |  5 +++++
 target-arm/translate.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+)

Changes in v7:
 - Break VMINNM/VMAXNM handling out into function
 - Use new softfloat routines for minnum/maxnum
 - Use extract32 to decode insn
 - Fix brace style

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3445813..7507240 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -4079,3 +4079,28 @@ float64 VFP_HELPER(muladd, d)(float64 a, float64 b, float64 c, void *fpstp)
     float_status *fpst = fpstp;
     return float64_muladd(a, b, c, 0, fpst);
 }
+
+/* ARMv8 VMAXNM/VMINNM */
+float32 VFP_HELPER(maxnm, s)(float32 a, float32 b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    return float32_maxnum(a, b, fpst);
+}
+
+float64 VFP_HELPER(maxnm, d)(float64 a, float64 b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    return float64_maxnum(a, b, fpst);
+}
+
+float32 VFP_HELPER(minnm, s)(float32 a, float32 b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    return float32_minnum(a, b, fpst);
+}
+
+float64 VFP_HELPER(minnm, d)(float64 a, float64 b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    return float64_minnum(a, b, fpst);
+}
diff --git a/target-arm/helper.h b/target-arm/helper.h
index cac9564..d459a39 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -132,6 +132,11 @@ DEF_HELPER_2(neon_fcvt_f32_to_f16, i32, f32, env)
 DEF_HELPER_4(vfp_muladdd, f64, f64, f64, f64, ptr)
 DEF_HELPER_4(vfp_muladds, f32, f32, f32, f32, ptr)
 
+DEF_HELPER_3(vfp_maxnmd, f64, f64, f64, ptr)
+DEF_HELPER_3(vfp_maxnms, f32, f32, f32, ptr)
+DEF_HELPER_3(vfp_minnmd, f64, f64, f64, ptr)
+DEF_HELPER_3(vfp_minnms, f32, f32, f32, ptr)
+
 DEF_HELPER_3(recps_f32, f32, f32, f32, env)
 DEF_HELPER_3(rsqrts_f32, f32, f32, f32, env)
 DEF_HELPER_2(recpe_f32, f32, f32, env)
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 0a22ad8..9a8069e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -2723,6 +2723,54 @@ static int handle_vsel(uint32_t insn, uint32_t rd, uint32_t rn, uint32_t rm,
     return 0;
 }
 
+static int handle_vminmaxnm(uint32_t insn, uint32_t rd, uint32_t rn,
+                            uint32_t rm, uint32_t dp)
+{
+    uint32_t vmin = extract32(insn, 6, 1);
+    TCGv_ptr fpst = get_fpstatus_ptr(0);
+
+    if (dp) {
+        TCGv_i64 frn, frm, dest;
+
+        frn = tcg_temp_new_i64();
+        frm = tcg_temp_new_i64();
+        dest = tcg_temp_new_i64();
+
+        tcg_gen_ld_f64(frn, cpu_env, vfp_reg_offset(dp, rn));
+        tcg_gen_ld_f64(frm, cpu_env, vfp_reg_offset(dp, rm));
+        if (vmin) {
+            gen_helper_vfp_minnmd(dest, frn, frm, fpst);
+        } else {
+            gen_helper_vfp_maxnmd(dest, frn, frm, fpst);
+        }
+        tcg_gen_st_f64(dest, cpu_env, vfp_reg_offset(dp, rd));
+        tcg_temp_free_i64(frn);
+        tcg_temp_free_i64(frm);
+        tcg_temp_free_i64(dest);
+    } else {
+        TCGv_i32 frn, frm, dest;
+
+        frn = tcg_temp_new_i32();
+        frm = tcg_temp_new_i32();
+        dest = tcg_temp_new_i32();
+
+        tcg_gen_ld_f32(frn, cpu_env, vfp_reg_offset(dp, rn));
+        tcg_gen_ld_f32(frm, cpu_env, vfp_reg_offset(dp, rm));
+        if (vmin) {
+            gen_helper_vfp_minnms(dest, frn, frm, fpst);
+        } else {
+            gen_helper_vfp_maxnms(dest, frn, frm, fpst);
+        }
+        tcg_gen_st_f32(dest, cpu_env, vfp_reg_offset(dp, rd));
+        tcg_temp_free_i32(frn);
+        tcg_temp_free_i32(frm);
+        tcg_temp_free_i32(dest);
+    }
+
+    tcg_temp_free_ptr(fpst);
+    return 0;
+}
+
 static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
 {
     uint32_t rd, rn, rm, dp = extract32(insn, 8, 1);
@@ -2743,6 +2791,8 @@ static int disas_vfp_v8_insn(CPUARMState *env, DisasContext *s, uint32_t insn)
 
     if ((insn & 0x0f800e50) == 0x0e000a00) {
         return handle_vsel(insn, rd, rn, rm, dp);
+    } else if ((insn & 0x0fb00e10) == 0x0e800a00) {
+        return handle_vminmaxnm(insn, rd, rn, rm, dp);
     }
     return 1;
 }
-- 
1.8.1.4

[Qemu-devel] [PATCH v7 6/6] target-arm: Implement ARMv8 SIMD VMAXNM and VMINNM instructions.

[Will Newton]

This adds support for the ARMv8 Advanced SIMD VMAXNM and VMINNM
instructions.

Signed-off-by: Will Newton <will.newton@linaro.org>
---
 target-arm/helper.h      |  3 +++
 target-arm/neon_helper.c | 16 ++++++++++++++++
 target-arm/translate.c   | 31 ++++++++++++++++++++++---------
 3 files changed, 41 insertions(+), 9 deletions(-)

Changes in v7:
 - Use new softfloat routines for minnum/maxnum
 - Rename NEON_3R_VRECPS_VRSQRTS to NEON_3R_FLOAT_MISC
 - Fix brace style

diff --git a/target-arm/helper.h b/target-arm/helper.h
index d459a39..3ecbbd2 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -355,6 +355,9 @@ DEF_HELPER_3(neon_cgt_f32, i32, i32, i32, ptr)
 DEF_HELPER_3(neon_acge_f32, i32, i32, i32, ptr)
 DEF_HELPER_3(neon_acgt_f32, i32, i32, i32, ptr)
 
+DEF_HELPER_3(neon_maxnm_f32, i32, i32, i32, ptr)
+DEF_HELPER_3(neon_minnm_f32, i32, i32, i32, ptr)
+
 /* iwmmxt_helper.c */
 DEF_HELPER_2(iwmmxt_maddsq, i64, i64, i64)
 DEF_HELPER_2(iwmmxt_madduq, i64, i64, i64)
diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
index b028cc2..06e6894 100644
--- a/target-arm/neon_helper.c
+++ b/target-arm/neon_helper.c
@@ -2008,3 +2008,19 @@ void HELPER(neon_zip16)(CPUARMState *env, uint32_t rd, uint32_t rm)
     env->vfp.regs[rm] = make_float64(m0);
     env->vfp.regs[rd] = make_float64(d0);
 }
+
+uint32_t HELPER(neon_maxnm_f32)(uint32_t a, uint32_t b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    float32 af = make_float32(a);
+    float32 bf = make_float32(b);
+    return float32_val(float32_maxnum(af, bf, fpst));
+}
+
+uint32_t HELPER(neon_minnm_f32)(uint32_t a, uint32_t b, void *fpstp)
+{
+    float_status *fpst = fpstp;
+    float32 af = make_float32(a);
+    float32 bf = make_float32(b);
+    return float32_val(float32_minnum(af, bf, fpst));
+}
diff --git a/target-arm/translate.c b/target-arm/translate.c
index 9a8069e..7d9213e 100644
--- a/target-arm/translate.c
+++ b/target-arm/translate.c
@@ -4553,7 +4553,7 @@ static void gen_neon_narrow_op(int op, int u, int size,
 #define NEON_3R_FLOAT_CMP 28 /* float VCEQ, VCGE, VCGT */
 #define NEON_3R_FLOAT_ACMP 29 /* float VACGE, VACGT, VACLE, VACLT */
 #define NEON_3R_FLOAT_MINMAX 30 /* float VMIN, VMAX */
-#define NEON_3R_VRECPS_VRSQRTS 31 /* float VRECPS, VRSQRTS */
+#define NEON_3R_FLOAT_MISC 31 /* float VRECPS, VRSQRTS, VMAXNM/MINNM */
 
 static const uint8_t neon_3r_sizes[] = {
     [NEON_3R_VHADD] = 0x7,
@@ -4586,7 +4586,7 @@ static const uint8_t neon_3r_sizes[] = {
     [NEON_3R_FLOAT_CMP] = 0x5, /* size bit 1 encodes op */
     [NEON_3R_FLOAT_ACMP] = 0x5, /* size bit 1 encodes op */
     [NEON_3R_FLOAT_MINMAX] = 0x5, /* size bit 1 encodes op */
-    [NEON_3R_VRECPS_VRSQRTS] = 0x5, /* size bit 1 encodes op */
+    [NEON_3R_FLOAT_MISC] = 0x5, /* size bit 1 encodes op */
 };
 
 /* Symbolic constants for op fields for Neon 2-register miscellaneous.
@@ -4847,8 +4847,9 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
                 return 1;
             }
             break;
-        case NEON_3R_VRECPS_VRSQRTS:
-            if (u) {
+        case NEON_3R_FLOAT_MISC:
+            /* VMAXNM/VMINNM in ARMv8 */
+            if (u && (!arm_feature(env, ARM_FEATURE_V8) || (size & 1))) {
                 return 1;
             }
             break;
@@ -5137,11 +5138,23 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
             tcg_temp_free_ptr(fpstatus);
             break;
         }
-        case NEON_3R_VRECPS_VRSQRTS:
-            if (size == 0)
-                gen_helper_recps_f32(tmp, tmp, tmp2, cpu_env);
-            else
-                gen_helper_rsqrts_f32(tmp, tmp, tmp2, cpu_env);
+        case NEON_3R_FLOAT_MISC:
+            if (u) {
+                /* VMAXNM/VMINNM */
+                TCGv_ptr fpstatus = get_fpstatus_ptr(1);
+                if (size == 0) {
+                    gen_helper_neon_maxnm_f32(tmp, tmp, tmp2, fpstatus);
+                } else {
+                    gen_helper_neon_minnm_f32(tmp, tmp, tmp2, fpstatus);
+                }
+                tcg_temp_free_ptr(fpstatus);
+            } else {
+                if (size == 0) {
+                    gen_helper_recps_f32(tmp, tmp, tmp2, cpu_env);
+                } else {
+                    gen_helper_rsqrts_f32(tmp, tmp, tmp2, cpu_env);
+              }
+            }
             break;
         case NEON_3R_VFM:
         {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v7 3/6] softfloat: Remove unused argument from MINMAX macro.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> The nan_exp argument is not used, so remove it.

Nice catch -- this has actually been present since I originally
introduced the macros (probably as cut-n-paste legacy from
the compare macros).

PS: you missed your signed-off-by line.

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

-- PMM

Re: [Qemu-devel] [PATCH v7 4/6] softfloat: Add minNum() and maxNum() functions to softfloat.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> Add floatnn_minnum() and floatnn_maxnum() functions which are equivalent
> to the minNum() and maxNum() functions from IEEE 754-2008. They are
> similar to min() and max() but differ in the handling of QNaN arguments.

Missing Signed-off-by: line here.

> ---
>  fpu/softfloat.c         | 54 +++++++++++++++++++++++++++++++++++++++++++++++++
>  include/fpu/softfloat.h |  4 ++++
>  2 files changed, 58 insertions(+)
>
> Changes in v7:
>  - New patch
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 97bf627..9834927 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -6750,6 +6750,60 @@ float ## s float ## s ## _max(float ## s a, float ## s b STATUS_PARAM)  \
>  MINMAX(32)
>  MINMAX(64)
>
> +/* minnum() and maxnum() functions. These are similar to the min()
> + * and max() functions but if one of the arguments is a QNaN and
> + * the other is numerical then the numerical argument is returned.

I think this comment could usefully be expanded to add:
 * minnum() and maxnum correspond to the IEEE 754-2008 minNum()
 * and maxNum() operations. min() and max() are the typical min/max
 * semantics provided by many CPUs which predate that specification.



> + */
> +#define MINMAXNUM(s)                                                       \
> +INLINE float ## s float ## s ## _minmaxnum(float ## s a, float ## s b,     \
> +                                           int ismin STATUS_PARAM )        \
> +{                                                                          \
> +    flag aSign, bSign;                                                     \
> +    uint ## s ## _t av, bv;                                                \
> +    a = float ## s ## _squash_input_denormal(a STATUS_VAR);                \
> +    b = float ## s ## _squash_input_denormal(b STATUS_VAR);                \
> +    if (float ## s ## _is_quiet_nan(a) &&                                  \
> +        !float ## s ##_is_quiet_nan(b)) {                                  \
> +        return b;                                                          \
> +    } else if (float ## s ## _is_quiet_nan(b) &&                           \
> +               !float ## s ## _is_quiet_nan(a)) {                          \
> +        return a;                                                          \

This is incorrect if the inputs are one signalling NaN and one
quiet NaN (both according to the IEEE spec and the ARM ARM
pseudocode). Among other things we will fail to signal InvalidOp.

Also, the bulk of this macro is identical to MINMAX. That suggests
we could instead extend MINMAX to define functions taking an
extra parameter isminmaxnum indicating we want 754-2008
semantics, and then we just need to replace the current NaN-check
clause in it with this one:

    if (float ## s ## _is_any_nan(a) ||
        float ## s ## _is_any_nan(b)) {
        if (isminmax) {
            if ((float ## s ## _is_quiet_nan(a) && !float ## s ##
_is_any_nan(b)) {
                return b;
            }
             if ((float ## s ## _is_quiet_nan(b) && !float ## s ##
_is_any_nan(a)) {
                return a;
            }
       }
       return propagateFloat ## s ## NaN(a, b STATUS_VAR);
  }

(line continuation backslashes omitted for clarity)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 1/6] target-arm: Move call to disas_vfp_insn out of disas_coproc_insn.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> Floating point is an extension to the instruction set rather than
> a coprocessor, so call it directly from the ARM and Thumb decode
> functions.

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

(In case you're not familiar with the convention, please copy
that reviewed-by line into the commit message for future versions
of this patch series, assuming you don't change the patch. That
tells me I've already looked at this patch and don't need to reread
it. You'll also need to add your own signed-off-by line.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 5/6] target-arm: Implement ARMv8 FP VMAXNM and VMINNM instructions.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> This adds support for the ARMv8 floating point VMAXNM and VMINNM
> instructions.

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

-- PMM

Re: [Qemu-devel] [PATCH v7 6/6] target-arm: Implement ARMv8 SIMD VMAXNM and VMINNM instructions.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> This adds support for the ARMv8 Advanced SIMD VMAXNM and VMINNM
> instructions.
>
> Signed-off-by: Will Newton <will.newton@linaro.org>
> ---
>  target-arm/helper.h      |  3 +++
>  target-arm/neon_helper.c | 16 ++++++++++++++++
>  target-arm/translate.c   | 31 ++++++++++++++++++++++---------
>  3 files changed, 41 insertions(+), 9 deletions(-)
>
> Changes in v7:
>  - Use new softfloat routines for minnum/maxnum
>  - Rename NEON_3R_VRECPS_VRSQRTS to NEON_3R_FLOAT_MISC
>  - Fix brace style
>
> diff --git a/target-arm/helper.h b/target-arm/helper.h
> index d459a39..3ecbbd2 100644
> --- a/target-arm/helper.h
> +++ b/target-arm/helper.h
> @@ -355,6 +355,9 @@ DEF_HELPER_3(neon_cgt_f32, i32, i32, i32, ptr)
>  DEF_HELPER_3(neon_acge_f32, i32, i32, i32, ptr)
>  DEF_HELPER_3(neon_acgt_f32, i32, i32, i32, ptr)
>
> +DEF_HELPER_3(neon_maxnm_f32, i32, i32, i32, ptr)
> +DEF_HELPER_3(neon_minnm_f32, i32, i32, i32, ptr)
> +
>  /* iwmmxt_helper.c */
>  DEF_HELPER_2(iwmmxt_maddsq, i64, i64, i64)
>  DEF_HELPER_2(iwmmxt_madduq, i64, i64, i64)
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index b028cc2..06e6894 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -2008,3 +2008,19 @@ void HELPER(neon_zip16)(CPUARMState *env, uint32_t rd, uint32_t rm)
>      env->vfp.regs[rm] = make_float64(m0);
>      env->vfp.regs[rd] = make_float64(d0);
>  }
> +
> +uint32_t HELPER(neon_maxnm_f32)(uint32_t a, uint32_t b, void *fpstp)
> +{
> +    float_status *fpst = fpstp;
> +    float32 af = make_float32(a);
> +    float32 bf = make_float32(b);
> +    return float32_val(float32_maxnum(af, bf, fpst));
> +}
> +
> +uint32_t HELPER(neon_minnm_f32)(uint32_t a, uint32_t b, void *fpstp)
> +{
> +    float_status *fpst = fpstp;
> +    float32 af = make_float32(a);
> +    float32 bf = make_float32(b);
> +    return float32_val(float32_minnum(af, bf, fpst));
> +}

These are still unnecessary duplicates of the helpers you've put in in
patch 5 -- just use those from the Neon instructions as well as the
VFP ones. The float32_val and make_float32 boxing/unboxing
macros are only so we get better typechecking from the C compiler,
so it is exactly equivalent to define the helper as taking and returning
float32, or to have it take and return uint32_t and then do the box/unbox
in the function. Just having it take/return float32 is shorter and easier
to read.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 9a8069e..7d9213e 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4553,7 +4553,7 @@ static void gen_neon_narrow_op(int op, int u, int size,
>  #define NEON_3R_FLOAT_CMP 28 /* float VCEQ, VCGE, VCGT */
>  #define NEON_3R_FLOAT_ACMP 29 /* float VACGE, VACGT, VACLE, VACLT */
>  #define NEON_3R_FLOAT_MINMAX 30 /* float VMIN, VMAX */
> -#define NEON_3R_VRECPS_VRSQRTS 31 /* float VRECPS, VRSQRTS */
> +#define NEON_3R_FLOAT_MISC 31 /* float VRECPS, VRSQRTS, VMAXNM/MINNM */
>
>  static const uint8_t neon_3r_sizes[] = {
>      [NEON_3R_VHADD] = 0x7,
> @@ -4586,7 +4586,7 @@ static const uint8_t neon_3r_sizes[] = {
>      [NEON_3R_FLOAT_CMP] = 0x5, /* size bit 1 encodes op */
>      [NEON_3R_FLOAT_ACMP] = 0x5, /* size bit 1 encodes op */
>      [NEON_3R_FLOAT_MINMAX] = 0x5, /* size bit 1 encodes op */
> -    [NEON_3R_VRECPS_VRSQRTS] = 0x5, /* size bit 1 encodes op */
> +    [NEON_3R_FLOAT_MISC] = 0x5, /* size bit 1 encodes op */
>  };
>
>  /* Symbolic constants for op fields for Neon 2-register miscellaneous.
> @@ -4847,8 +4847,9 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
>                  return 1;
>              }
>              break;
> -        case NEON_3R_VRECPS_VRSQRTS:
> -            if (u) {
> +        case NEON_3R_FLOAT_MISC:
> +            /* VMAXNM/VMINNM in ARMv8 */
> +            if (u && (!arm_feature(env, ARM_FEATURE_V8) || (size & 1))) {

I don't think we need to check size bit 0 here, do we?
The check against the entry in neon_3r_sizes[] above should
only pass if size is 0 or 2.

>                  return 1;
>              }
>              break;
> @@ -5137,11 +5138,23 @@ static int disas_neon_data_insn(CPUARMState * env, DisasContext *s, uint32_t ins
>              tcg_temp_free_ptr(fpstatus);
>              break;
>          }
> -        case NEON_3R_VRECPS_VRSQRTS:
> -            if (size == 0)
> -                gen_helper_recps_f32(tmp, tmp, tmp2, cpu_env);
> -            else
> -                gen_helper_rsqrts_f32(tmp, tmp, tmp2, cpu_env);
> +        case NEON_3R_FLOAT_MISC:
> +            if (u) {
> +                /* VMAXNM/VMINNM */
> +                TCGv_ptr fpstatus = get_fpstatus_ptr(1);
> +                if (size == 0) {
> +                    gen_helper_neon_maxnm_f32(tmp, tmp, tmp2, fpstatus);
> +                } else {
> +                    gen_helper_neon_minnm_f32(tmp, tmp, tmp2, fpstatus);
> +                }
> +                tcg_temp_free_ptr(fpstatus);
> +            } else {
> +                if (size == 0) {
> +                    gen_helper_recps_f32(tmp, tmp, tmp2, cpu_env);
> +                } else {
> +                    gen_helper_rsqrts_f32(tmp, tmp, tmp2, cpu_env);
> +              }
> +            }
>              break;
>          case NEON_3R_VFM:
>          {

thanks
-- PMM

Re: [Qemu-devel] [PATCH v7 2/6] target-arm: Implement ARMv8 VSEL instruction.

[Peter Maydell]

On 2 December 2013 20:12, Will Newton <will.newton@linaro.org> wrote:
> This adds support for the VSEL floating point selection instruction
> which was added in ARMv8.

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

thanks
-- PMM