diff mbox series

[4/5] tricore: add QSEED instruction

Message ID 20190605061126.10244-5-david.brenken@efs-auto.org
State New
Headers show
Series tricore: adding new instructions and fixing issues | expand

Commit Message

David Brenken June 5, 2019, 6:11 a.m. UTC 
From: Andreas Konopik <andreas.konopik@efs-auto.com>

Signed-off-by: Andreas Konopik <andreas.konopik@efs-auto.de>
Signed-off-by: David Brenken <david.brenken@efs-auto.de>
Signed-off-by: Georg Hofstetter <georg.hofstetter@efs-auto.de>
Signed-off-by: Robert Rasche <robert.rasche@efs-auto.de>
Signed-off-by: Lars Biermanski <lars.biermanski@efs-auto.de>

---
 target/tricore/fpu_helper.c | 88 +++++++++++++++++++++++++++++++++++++
 target/tricore/helper.h     |  1 +
 target/tricore/translate.c  |  3 ++
 3 files changed, 92 insertions(+)

Comments

Bastian Koppelmann June 5, 2019, 3:04 p.m. UTC | #1 
Hi,

On 6/5/19 8:11 AM, David Brenken wrote:
> +/*
> + * Target TriCore QSEED.F significand Lookup Table
> + *
> + * The QSEED.F output significand depends on the least-significant
> + * exponent bit and the 6 most-significant significand bits.
> + *
> + * IEEE 754 float datatype
> + * partitioned into Sign (S), Exponent (E) and Significand (M):
> + *
> + * S   E E E E E E E E   M M M M M M ...
> + *    |             |               |
> + *    +------+------+-------+-------+
> + *           |              |
> + *          for        lookup table
> + *      calculating     index for
> + *        output E       output M
> + */
> +static const uint8_t target_qseed_significand_table[128] = {
> +    253, 252, 245, 244, 239, 238, 231, 230, 225, 224, 217, 216,
> +    211, 210, 205, 204, 201, 200, 195, 194, 189, 188, 185, 184,
> +    179, 178, 175, 174, 169, 168, 165, 164, 161, 160, 157, 156,
> +    153, 152, 149, 148, 145, 144, 141, 140, 137, 136, 133, 132,
> +    131, 130, 127, 126, 123, 122, 121, 120, 117, 116, 115, 114,
> +    111, 110, 109, 108, 103, 102, 99, 98, 93, 92, 89, 88, 83,
> +    82, 79, 78, 75, 74, 71, 70, 67, 66, 63, 62, 59, 58, 55,
> +    54, 53, 52, 49, 48, 45, 44, 43, 42, 39, 38, 37, 36, 33,
> +    32, 31, 30, 27, 26, 25, 24, 23, 22, 19, 18, 17, 16, 15,
> +    14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2
> +};


Can you explain in a comment how you arrived at this lookup table?


> +    } else if (float32_is_neg(arg1)) {
> +        result = float32_sqrt_nan;
> +        env->FPU_FI = 1;
[...]
> +
> +    flags = f_get_excp_flags(env);
> +    if (flags) {
> +        if (flags & float_flag_invalid) {
> +            f_update_psw_flags(env, flags);
> +        } else {
> +            env->FPU_FS = 0;
> +        }
> +    } else {
> +        env->FPU_FS = 0;

You are setting FPU_FS to 0, even though FPU_FI might have been set in 
case of a NaN. I think it's best to remove the whole softfloat check, as 
none of the softfloat functions you call, can raise any flags.

Cheers,

Bastian
Konopik, Andreas (EFS-GH2) June 7, 2019, 8:40 a.m. UTC | #2 
Hi Bastian,

> Hi,
> 
> On 6/5/19 8:11 AM, David Brenken wrote:
> > +/*
> > + * Target TriCore QSEED.F significand Lookup Table
> > + *
> > + * The QSEED.F output significand depends on the least-significant
> > + * exponent bit and the 6 most-significant significand bits.
> > + *
> > + * IEEE 754 float datatype
> > + * partitioned into Sign (S), Exponent (E) and Significand (M):
> > + *
> > + * S   E E E E E E E E   M M M M M M ...
> > + *    |             |               |
> > + *    +------+------+-------+-------+
> > + *           |              |
> > + *          for        lookup table
> > + *      calculating     index for
> > + *        output E       output M
> > + */
> > +static const uint8_t target_qseed_significand_table[128] = {
> > +    253, 252, 245, 244, 239, 238, 231, 230, 225, 224, 217, 216,
> > +    211, 210, 205, 204, 201, 200, 195, 194, 189, 188, 185, 184,
> > +    179, 178, 175, 174, 169, 168, 165, 164, 161, 160, 157, 156,
> > +    153, 152, 149, 148, 145, 144, 141, 140, 137, 136, 133, 132,
> > +    131, 130, 127, 126, 123, 122, 121, 120, 117, 116, 115, 114,
> > +    111, 110, 109, 108, 103, 102, 99, 98, 93, 92, 89, 88, 83,
> > +    82, 79, 78, 75, 74, 71, 70, 67, 66, 63, 62, 59, 58, 55,
> > +    54, 53, 52, 49, 48, 45, 44, 43, 42, 39, 38, 37, 36, 33,
> > +    32, 31, 30, 27, 26, 25, 24, 23, 22, 19, 18, 17, 16, 15,
> > +    14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2 };
> 
> 
> Can you explain in a comment how you arrived at this lookup table?

We extracted this lookup table from real hardware results.
We also validated QEMU output values by comparing again with the real
hardware qseed output.
Testing was not made for all 2^32 possible input values,but for all
positive floats that yield different outputs (with a big stride).

We will add a shorter comment to the second version of the patchset

> > +    } else if (float32_is_neg(arg1)) {
> > +        result = float32_sqrt_nan;
> > +        env->FPU_FI = 1;
> [...]
> > +
> > +    flags = f_get_excp_flags(env);
> > +    if (flags) {
> > +        if (flags & float_flag_invalid) {
> > +            f_update_psw_flags(env, flags);
> > +        } else {
> > +            env->FPU_FS = 0;
> > +        }
> > +    } else {
> > +        env->FPU_FS = 0;
> 
> You are setting FPU_FS to 0, even though FPU_FI might have been set in case of
> a NaN. I think it's best to remove the whole softfloat check, as none of the
> softfloat functions you call, can raise any flags.

I will make the PSW update only dependent on is_NaN(input) and is_SQRT_NaN(output).

Best regards,

Andreas
diff mbox series

Patch

diff --git a/target/tricore/fpu_helper.c b/target/tricore/fpu_helper.c
index 432079c8e2..68515ee3e0 100644
--- a/target/tricore/fpu_helper.c
+++ b/target/tricore/fpu_helper.c
@@ -24,6 +24,7 @@ 
 
 #define QUIET_NAN 0x7fc00000
 #define ADD_NAN   0x7fc00001
+#define SQRT_NAN  0x7fc00004
 #define DIV_NAN   0x7fc00008
 #define MUL_NAN   0x7fc00002
 #define FPU_FS PSW_USB_C
@@ -32,6 +33,9 @@ 
 #define FPU_FZ PSW_USB_AV
 #define FPU_FU PSW_USB_SAV
 
+#define float32_sqrt_nan make_float32(SQRT_NAN)
+#define float32_quiet_nan make_float32(QUIET_NAN)
+
 /* we don't care about input_denormal */
 static inline uint8_t f_get_excp_flags(CPUTriCoreState *env)
 {
@@ -166,6 +170,90 @@  uint32_t helper_fmul(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
 
 }
 
+/*
+ * Target TriCore QSEED.F significand Lookup Table
+ *
+ * The QSEED.F output significand depends on the least-significant
+ * exponent bit and the 6 most-significant significand bits.
+ *
+ * IEEE 754 float datatype
+ * partitioned into Sign (S), Exponent (E) and Significand (M):
+ *
+ * S   E E E E E E E E   M M M M M M ...
+ *    |             |               |
+ *    +------+------+-------+-------+
+ *           |              |
+ *          for        lookup table
+ *      calculating     index for
+ *        output E       output M
+ */
+static const uint8_t target_qseed_significand_table[128] = {
+    253, 252, 245, 244, 239, 238, 231, 230, 225, 224, 217, 216,
+    211, 210, 205, 204, 201, 200, 195, 194, 189, 188, 185, 184,
+    179, 178, 175, 174, 169, 168, 165, 164, 161, 160, 157, 156,
+    153, 152, 149, 148, 145, 144, 141, 140, 137, 136, 133, 132,
+    131, 130, 127, 126, 123, 122, 121, 120, 117, 116, 115, 114,
+    111, 110, 109, 108, 103, 102, 99, 98, 93, 92, 89, 88, 83,
+    82, 79, 78, 75, 74, 71, 70, 67, 66, 63, 62, 59, 58, 55,
+    54, 53, 52, 49, 48, 45, 44, 43, 42, 39, 38, 37, 36, 33,
+    32, 31, 30, 27, 26, 25, 24, 23, 22, 19, 18, 17, 16, 15,
+    14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2
+};
+
+uint32_t helper_qseed(CPUTriCoreState *env, uint32_t r1)
+{
+    uint32_t flags;
+    uint32_t arg1, S, E, M, E_minus_one, m_idx;
+    uint32_t new_E, new_M, new_S, result;
+
+    arg1 = make_float32(r1);
+
+    /* fetch IEEE-754 fields S, E and the uppermost 6-bit of M */
+    S = extract32(arg1, 31, 1);
+    E = extract32(arg1, 23, 8);
+    M = extract32(arg1, 17, 6);
+
+    if (float32_is_any_nan(arg1)) {
+        result = float32_quiet_nan;
+        env->FPU_FI = 1;
+    } else if (E == 0) {
+        if (float32_is_neg(arg1)) {
+            result = float32_infinity | (1 << 31);
+        } else {
+            result = float32_infinity;
+        }
+    } else if (float32_is_neg(arg1)) {
+        result = float32_sqrt_nan;
+        env->FPU_FI = 1;
+    } else if (float32_is_infinity(arg1)) {
+        result = float32_zero;
+    } else {
+        E_minus_one = E - 1;
+        m_idx = ((E_minus_one & 1) << 6) | M;
+        new_S = S;
+        new_E = 0xBD - E_minus_one / 2;
+        new_M = target_qseed_significand_table[m_idx];
+
+        result = 0;
+        result = deposit32(result, 31, 1, new_S);
+        result = deposit32(result, 23, 8, new_E);
+        result = deposit32(result, 15, 8, new_M);
+    }
+
+    flags = f_get_excp_flags(env);
+    if (flags) {
+        if (flags & float_flag_invalid) {
+            f_update_psw_flags(env, flags);
+        } else {
+            env->FPU_FS = 0;
+        }
+    } else {
+        env->FPU_FS = 0;
+    }
+
+    return (uint32_t) result;
+}
+
 uint32_t helper_fdiv(CPUTriCoreState *env, uint32_t r1, uint32_t r2)
 {
     uint32_t flags;
diff --git a/target/tricore/helper.h b/target/tricore/helper.h
index f1a5cb367e..b64780c37d 100644
--- a/target/tricore/helper.h
+++ b/target/tricore/helper.h
@@ -109,6 +109,7 @@  DEF_HELPER_3(fdiv, i32, env, i32, i32)
 DEF_HELPER_4(fmadd, i32, env, i32, i32, i32)
 DEF_HELPER_4(fmsub, i32, env, i32, i32, i32)
 DEF_HELPER_3(fcmp, i32, env, i32, i32)
+DEF_HELPER_2(qseed, i32, env, i32)
 DEF_HELPER_2(ftoi, i32, env, i32)
 DEF_HELPER_2(itof, i32, env, i32)
 DEF_HELPER_2(utof, i32, env, i32)
diff --git a/target/tricore/translate.c b/target/tricore/translate.c
index 19d8db7a46..dbc964901f 100644
--- a/target/tricore/translate.c
+++ b/target/tricore/translate.c
@@ -6764,6 +6764,9 @@  static void decode_rr_divide(CPUTriCoreState *env, DisasContext *ctx)
     case OPC2_32_RR_UPDFL:
         gen_helper_updfl(cpu_env, cpu_gpr_d[r1]);
         break;
+    case OPC2_32_RR_QSEED_F:
+        gen_helper_qseed(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
+        break;
     case OPC2_32_RR_UTOF:
         gen_helper_utof(cpu_gpr_d[r3], cpu_env, cpu_gpr_d[r1]);
         break;