[PATCH 04/21] target/mips: Explicitly set 2-NaN propagation rule

Peter Maydell posted 21 patches 4 weeks, 1 day ago
[PATCH 04/21] target/mips: Explicitly set 2-NaN propagation rule
Posted by Peter Maydell 4 weeks, 1 day ago
Set the 2-NaN propagation rule explicitly in the float_status words
we use.

For active_fpu.fp_status, we do this in a new fp_reset() function
which mirrors the existing msa_reset() function in doing "first call
restore to set the fp status parts that depend on CPU state, then set
the fp status parts that are constant".

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/mips/fpu_helper.h       | 22 ++++++++++++++++++++++
 target/mips/cpu.c              |  2 +-
 target/mips/msa.c              | 17 +++++++++++++++++
 fpu/softfloat-specialize.c.inc | 18 ++----------------
 4 files changed, 42 insertions(+), 17 deletions(-)

diff --git a/target/mips/fpu_helper.h b/target/mips/fpu_helper.h
index ad1116e8c10..7c3c7897b45 100644
--- a/target/mips/fpu_helper.h
+++ b/target/mips/fpu_helper.h
@@ -44,6 +44,28 @@ static inline void restore_fp_status(CPUMIPSState *env)
     restore_snan_bit_mode(env);
 }
 
+static inline void fp_reset(CPUMIPSState *env)
+{
+    restore_fp_status(env);
+
+    /*
+     * According to MIPS specifications, if one of the two operands is
+     * a sNaN, a new qNaN has to be generated. This is done in
+     * floatXX_silence_nan(). For qNaN inputs the specifications
+     * says: "When possible, this QNaN result is one of the operand QNaN
+     * values." In practice it seems that most implementations choose
+     * the first operand if both operands are qNaN. In short this gives
+     * the following rules:
+     *  1. A if it is signaling
+     *  2. B if it is signaling
+     *  3. A (quiet)
+     *  4. B (quiet)
+     * A signaling NaN is always silenced before returning it.
+     */
+    set_float_2nan_prop_rule(float_2nan_prop_s_ab,
+                             &env->active_fpu.fp_status);
+}
+
 /* MSA */
 
 enum CPUMIPSMSADataFormat {
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index 9724e71a5e0..d0a43b6d5c7 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -407,9 +407,9 @@ static void mips_cpu_reset_hold(Object *obj, ResetType type)
     }
 
     msa_reset(env);
+    fp_reset(env);
 
     compute_hflags(env);
-    restore_fp_status(env);
     restore_pamask(env);
     cs->exception_index = EXCP_NONE;
 
diff --git a/target/mips/msa.c b/target/mips/msa.c
index 61f1a9a5936..9dffc428f5c 100644
--- a/target/mips/msa.c
+++ b/target/mips/msa.c
@@ -49,6 +49,23 @@ void msa_reset(CPUMIPSState *env)
     set_float_detect_tininess(float_tininess_after_rounding,
                               &env->active_tc.msa_fp_status);
 
+    /*
+     * According to MIPS specifications, if one of the two operands is
+     * a sNaN, a new qNaN has to be generated. This is done in
+     * floatXX_silence_nan(). For qNaN inputs the specifications
+     * says: "When possible, this QNaN result is one of the operand QNaN
+     * values." In practice it seems that most implementations choose
+     * the first operand if both operands are qNaN. In short this gives
+     * the following rules:
+     *  1. A if it is signaling
+     *  2. B if it is signaling
+     *  3. A (quiet)
+     *  4. B (quiet)
+     * A signaling NaN is always silenced before returning it.
+     */
+    set_float_2nan_prop_rule(float_2nan_prop_s_ab,
+                             &env->active_tc.msa_fp_status);
+
     /* clear float_status exception flags */
     set_float_exception_flags(0, &env->active_tc.msa_fp_status);
 
diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index 70cd3628b54..c60b999aa3d 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -402,24 +402,10 @@ static int pickNaN(FloatClass a_cls, FloatClass b_cls,
         /* target didn't set the rule: fall back to old ifdef choices */
 #if defined(TARGET_AVR) || defined(TARGET_HEXAGON) \
     || defined(TARGET_RISCV) || defined(TARGET_SH4) \
-    || defined(TARGET_TRICORE) || defined(TARGET_ARM)
+    || defined(TARGET_TRICORE) || defined(TARGET_ARM) || defined(TARGET_MIPS)
         g_assert_not_reached();
-#elif defined(TARGET_MIPS) || defined(TARGET_HPPA) || \
+#elif defined(TARGET_HPPA) || \
     defined(TARGET_LOONGARCH64) || defined(TARGET_S390X)
-        /*
-         * According to MIPS specifications, if one of the two operands is
-         * a sNaN, a new qNaN has to be generated. This is done in
-         * floatXX_silence_nan(). For qNaN inputs the specifications
-         * says: "When possible, this QNaN result is one of the operand QNaN
-         * values." In practice it seems that most implementations choose
-         * the first operand if both operands are qNaN. In short this gives
-         * the following rules:
-         *  1. A if it is signaling
-         *  2. B if it is signaling
-         *  3. A (quiet)
-         *  4. B (quiet)
-         * A signaling NaN is always silenced before returning it.
-         */
         rule = float_2nan_prop_s_ab;
 #elif defined(TARGET_PPC) || defined(TARGET_M68K)
         /*
-- 
2.34.1
Re: [PATCH 04/21] target/mips: Explicitly set 2-NaN propagation rule
Posted by Philippe Mathieu-Daudé 4 weeks ago
On 25/10/24 11:12, Peter Maydell wrote:
> Set the 2-NaN propagation rule explicitly in the float_status words
> we use.
> 
> For active_fpu.fp_status, we do this in a new fp_reset() function
> which mirrors the existing msa_reset() function in doing "first call
> restore to set the fp status parts that depend on CPU state, then set
> the fp status parts that are constant".
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/mips/fpu_helper.h       | 22 ++++++++++++++++++++++
>   target/mips/cpu.c              |  2 +-
>   target/mips/msa.c              | 17 +++++++++++++++++
>   fpu/softfloat-specialize.c.inc | 18 ++----------------
>   4 files changed, 42 insertions(+), 17 deletions(-)
> 
> diff --git a/target/mips/fpu_helper.h b/target/mips/fpu_helper.h
> index ad1116e8c10..7c3c7897b45 100644
> --- a/target/mips/fpu_helper.h
> +++ b/target/mips/fpu_helper.h
> @@ -44,6 +44,28 @@ static inline void restore_fp_status(CPUMIPSState *env)
>       restore_snan_bit_mode(env);
>   }
>   
> +static inline void fp_reset(CPUMIPSState *env)
> +{
> +    restore_fp_status(env);
> +
> +    /*
> +     * According to MIPS specifications, if one of the two operands is
> +     * a sNaN, a new qNaN has to be generated. This is done in
> +     * floatXX_silence_nan(). For qNaN inputs the specifications
> +     * says: "When possible, this QNaN result is one of the operand QNaN
> +     * values." In practice it seems that most implementations choose
> +     * the first operand if both operands are qNaN. In short this gives
> +     * the following rules:
> +     *  1. A if it is signaling
> +     *  2. B if it is signaling
> +     *  3. A (quiet)
> +     *  4. B (quiet)
> +     * A signaling NaN is always silenced before returning it.
> +     */
> +    set_float_2nan_prop_rule(float_2nan_prop_s_ab,
> +                             &env->active_fpu.fp_status);
> +}
> +
>   /* MSA */
>   
>   enum CPUMIPSMSADataFormat {
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index 9724e71a5e0..d0a43b6d5c7 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -407,9 +407,9 @@ static void mips_cpu_reset_hold(Object *obj, ResetType type)
>       }
>   
>       msa_reset(env);
> +    fp_reset(env);
>   
>       compute_hflags(env);
> -    restore_fp_status(env);
>       restore_pamask(env);
>       cs->exception_index = EXCP_NONE;
>   
> diff --git a/target/mips/msa.c b/target/mips/msa.c
> index 61f1a9a5936..9dffc428f5c 100644
> --- a/target/mips/msa.c
> +++ b/target/mips/msa.c
> @@ -49,6 +49,23 @@ void msa_reset(CPUMIPSState *env)
>       set_float_detect_tininess(float_tininess_after_rounding,
>                                 &env->active_tc.msa_fp_status);
>   
> +    /*
> +     * According to MIPS specifications, if one of the two operands is
> +     * a sNaN, a new qNaN has to be generated. This is done in
> +     * floatXX_silence_nan(). For qNaN inputs the specifications
> +     * says: "When possible, this QNaN result is one of the operand QNaN
> +     * values." In practice it seems that most implementations choose
> +     * the first operand if both operands are qNaN. In short this gives
> +     * the following rules:
> +     *  1. A if it is signaling
> +     *  2. B if it is signaling
> +     *  3. A (quiet)
> +     *  4. B (quiet)
> +     * A signaling NaN is always silenced before returning it.
> +     */
> +    set_float_2nan_prop_rule(float_2nan_prop_s_ab,
> +                             &env->active_tc.msa_fp_status);

Alternatively pass float_status* to fp_reset() and call it here as

        mips_fp_reset(&env->active_tc.msa_fp_status);

So we keep the comment in one place.

Regardless,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>