diff mbox

[v5,5/9] target-mips: Activate IEEE 274-2008 signaling NaN bit meaning

Message ID 1460995422-14373-6-git-send-email-aleksandar.markovic@rt-rk.com (mailing list archive)
State New, archived
Headers show

Commit Message

Aleksandar Markovic April 18, 2016, 4:03 p.m. UTC
From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>

Functions mips_cpu_reset() and msa_reset() are updated so that flag
snan_bit_is_one is properly set for any Mips FPU/MSA configuration.
For main FPUs, CPUs with FCR31's FCR31_NAN2008 bit set will invoke
set_snan_bit_is_one(0). For MSA, as it is IEEE 274-2008 compliant
from it inception, set_snan_bit_is_one(0) will always be invoked.

By applying this patch, a number of incorrect behaviors for CPU
configurations that require IEEE 274-2008 compliance will be fixed.
Those are behaviors that (up to the moment of applying this patch)
did not get the desired functionality from SoftFloat library with
respect to distinguishing between quiet and signaling NaN, getting
default NaN values (both quiet and signaling), establishing if a
floating point number is Nan or not, etc.

Just two examples:

* <MAX|MAXA>.<D|S> will now correctly detect and propagate NaNs.
* CLASS.<D|S> will now correcty detect NaN flavors, both their
  CPU FPU and MSA version.

Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
---
 target-mips/translate.c      | 6 +++++-
 target-mips/translate_init.c | 3 ++-
 2 files changed, 7 insertions(+), 2 deletions(-)

Comments

Maciej W. Rozycki April 25, 2016, 2:06 p.m. UTC | #1
On Mon, 18 Apr 2016, Aleksandar Markovic wrote:

> Functions mips_cpu_reset() and msa_reset() are updated so that flag
> snan_bit_is_one is properly set for any Mips FPU/MSA configuration.
> For main FPUs, CPUs with FCR31's FCR31_NAN2008 bit set will invoke
> set_snan_bit_is_one(0). For MSA, as it is IEEE 274-2008 compliant
> from it inception, set_snan_bit_is_one(0) will always be invoked.

 I have skimmed over the series -- have you lost MIPSr3 support (writable 
FCSR ABS2008 and NAN2008 bits) in porting?

  Maciej
Aleksandar Markovic April 25, 2016, 5:10 p.m. UTC | #2
No, nothing is lost. The plan is to add this functionality at a later time.
Maciej W. Rozycki April 29, 2016, 2:21 p.m. UTC | #3
On Mon, 25 Apr 2016, Aleksandar Markovic wrote:

> No, nothing is lost. The plan is to add this functionality at a later time.

 OK then, as you prefer.  Although I find the order somewhat odd as r5+ is 
a special case of r3.

  Maciej
Leon Alrae April 29, 2016, 3:06 p.m. UTC | #4
On 18/04/16 17:03, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> 
> Functions mips_cpu_reset() and msa_reset() are updated so that flag
> snan_bit_is_one is properly set for any Mips FPU/MSA configuration.
> For main FPUs, CPUs with FCR31's FCR31_NAN2008 bit set will invoke
> set_snan_bit_is_one(0). For MSA, as it is IEEE 274-2008 compliant
> from it inception, set_snan_bit_is_one(0) will always be invoked.
> 
> By applying this patch, a number of incorrect behaviors for CPU
> configurations that require IEEE 274-2008 compliance will be fixed.
> Those are behaviors that (up to the moment of applying this patch)
> did not get the desired functionality from SoftFloat library with
> respect to distinguishing between quiet and signaling NaN, getting
> default NaN values (both quiet and signaling), establishing if a
> floating point number is Nan or not, etc.
> 
> Just two examples:
> 
> * <MAX|MAXA>.<D|S> will now correctly detect and propagate NaNs.
> * CLASS.<D|S> will now correcty detect NaN flavors, both their
>   CPU FPU and MSA version.
> 
> Signed-off-by: Thomas Schwinge <thomas@codesourcery.com>
> Signed-off-by: Maciej W. Rozycki <macro@codesourcery.com>
> Signed-off-by: Aleksandar Markovic <aleksandar.markovic@imgtec.com>
> ---
>  target-mips/translate.c      | 6 +++++-
>  target-mips/translate_init.c | 3 ++-
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index e934884..2cdd2bd 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -20129,7 +20129,11 @@ void cpu_state_reset(CPUMIPSState *env)
>      env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
>      env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
>      env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
> -    set_snan_bit_is_one(1, &env->active_fpu.fp_status);
> +    if ((env->active_fpu.fcr31 >> FCR31_NAN2008) & 1) {
> +        set_snan_bit_is_one(0, &env->active_fpu.fp_status);
> +    } else {
> +        set_snan_bit_is_one(1, &env->active_fpu.fp_status);
> +    }
>      env->msair = env->cpu_model->MSAIR;
>      env->insn_flags = env->cpu_model->insn_flags;
>  
> diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
> index 1094baa..bae6183 100644
> --- a/target-mips/translate_init.c
> +++ b/target-mips/translate_init.c
> @@ -904,5 +904,6 @@ static void msa_reset(CPUMIPSState *env)
>      /* clear float_status nan mode */
>      set_default_nan_mode(0, &env->active_tc.msa_fp_status);
>  
> -    set_snan_bit_is_one(1, &env->active_tc.msa_fp_status);
> +    /* set proper signanling bit meaning ("1" means "quiet") */
> +    set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
>  }

To support r3, specifically writable {NAN,ABS}2008 bits, we will need to
restore snan_bit_is_one in more places than just reset (for example
after migration), which suggests that the code in this patch deserves to
be placed in a separate function, just like it was done originally.
Also, having the fcr31_rw_bitmask would nicely clean up the fcr31
handling in helper_ctc1.

If you plan to do that later then that's OK as far as I'm concerned, but
if those changes (which were already posted and not that big) were
included here from the beginning then we would avoid having to rework
above code.

Thanks,
Leon
diff mbox

Patch

diff --git a/target-mips/translate.c b/target-mips/translate.c
index e934884..2cdd2bd 100644
--- a/target-mips/translate.c
+++ b/target-mips/translate.c
@@ -20129,7 +20129,11 @@  void cpu_state_reset(CPUMIPSState *env)
     env->CP0_PageGrain = env->cpu_model->CP0_PageGrain;
     env->active_fpu.fcr0 = env->cpu_model->CP1_fcr0;
     env->active_fpu.fcr31 = env->cpu_model->CP1_fcr31;
-    set_snan_bit_is_one(1, &env->active_fpu.fp_status);
+    if ((env->active_fpu.fcr31 >> FCR31_NAN2008) & 1) {
+        set_snan_bit_is_one(0, &env->active_fpu.fp_status);
+    } else {
+        set_snan_bit_is_one(1, &env->active_fpu.fp_status);
+    }
     env->msair = env->cpu_model->MSAIR;
     env->insn_flags = env->cpu_model->insn_flags;
 
diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c
index 1094baa..bae6183 100644
--- a/target-mips/translate_init.c
+++ b/target-mips/translate_init.c
@@ -904,5 +904,6 @@  static void msa_reset(CPUMIPSState *env)
     /* clear float_status nan mode */
     set_default_nan_mode(0, &env->active_tc.msa_fp_status);
 
-    set_snan_bit_is_one(1, &env->active_tc.msa_fp_status);
+    /* set proper signanling bit meaning ("1" means "quiet") */
+    set_snan_bit_is_one(0, &env->active_tc.msa_fp_status);
 }