[Qemu-devel] [PATCH 0/4] target-arm: Fix VRSQRTS

[Qemu-devel] [PATCH 0/4] target-arm: Fix VRSQRTS

[Peter Maydell]

This patch series fixes various bugs in VRSQRTS including the
major one that it was calculating completely the wrong value
(missing a division by 2).

It also introduces the infrastructure to support the ARM
"Standard FPSCR Value" as used for operations which use "ARM
standard floating-point arithmetic" rather than being controlled
by the rounding/flush/NaN settings in the FPSCR. (Most Neon FP
instructions should be using this, so there will be subsequent
patches to use it with other instructions too.)

The "missing divide" bug was fixed in meego-qemu commit 438a549e0;
the remaining fixes here are new.

Tested in the usual random-instruction-generation fashion.

Peter Maydell (4):
  softfloat: Add float32_is_zero_or_denormal() function
  target-arm: Fix implementation of VRSQRTS
  target-arm: Add support for 'Standard FPSCR Value' as used by Neon
  target-arm: Use the standard FPSCR value for VRSQRTS

 fpu/softfloat.h     |    5 +++++
 target-arm/cpu.h    |   13 +++++++++++++
 target-arm/helper.c |   17 +++++++++++++++--
 3 files changed, 33 insertions(+), 2 deletions(-)

[Qemu-devel] [PATCH 1/4] softfloat: Add float32_is_zero_or_denormal() function

[Peter Maydell]

Add a utility function to softfloat to test whether a float32
is zero or denormal.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 fpu/softfloat.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fpu/softfloat.h b/fpu/softfloat.h
index 15052cc..1acf7af 100644
--- a/fpu/softfloat.h
+++ b/fpu/softfloat.h
@@ -336,6 +336,11 @@ INLINE int float32_is_any_nan(float32 a)
     return ((float32_val(a) & ~(1 << 31)) > 0x7f800000UL);
 }
 
+INLINE int float32_is_zero_or_denormal(float32 a)
+{
+    return (float32_val(a) & 0x7f800000) == 0;
+}
+
 #define float32_zero make_float32(0)
 #define float32_one make_float32(0x3f800000)
 #define float32_ln2 make_float32(0x3f317218)
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 1/4] softfloat: Add float32_is_zero_or_denormal() function

[Aurelien Jarno]

On Fri, Jan 07, 2011 at 09:26:24AM +0000, Peter Maydell wrote:
> Add a utility function to softfloat to test whether a float32
> is zero or denormal.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  fpu/softfloat.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

> diff --git a/fpu/softfloat.h b/fpu/softfloat.h
> index 15052cc..1acf7af 100644
> --- a/fpu/softfloat.h
> +++ b/fpu/softfloat.h
> @@ -336,6 +336,11 @@ INLINE int float32_is_any_nan(float32 a)
>      return ((float32_val(a) & ~(1 << 31)) > 0x7f800000UL);
>  }
>  
> +INLINE int float32_is_zero_or_denormal(float32 a)
> +{
> +    return (float32_val(a) & 0x7f800000) == 0;
> +}
> +
>  #define float32_zero make_float32(0)
>  #define float32_one make_float32(0x3f800000)
>  #define float32_ln2 make_float32(0x3f317218)
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 2/4] target-arm: Fix implementation of VRSQRTS

[Peter Maydell]

The implementation of the ARM VRSQRTS instruction (which calculates
(3 - op1 * op2) / 2) was missing the division operation. It also
did not handle the special cases of (0,inf) and (inf,0).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 705b99f..ac47de0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2612,8 +2612,16 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env)
 float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env)
 {
     float_status *s = &env->vfp.fp_status;
+    float32 two = int32_to_float32(2, s);
     float32 three = int32_to_float32(3, s);
-    return float32_sub(three, float32_mul(a, b, s), s);
+    float32 product;
+    if ((float32_is_infinity(a) && float32_is_zero_or_denormal(b)) ||
+        (float32_is_infinity(b) && float32_is_zero_or_denormal(a))) {
+        product = float32_zero;
+    } else {
+        product = float32_mul(a, b, s);
+    }
+    return float32_div(float32_sub(three, product, s), two, s);
 }
 
 /* NEON helpers.  */
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 2/4] target-arm: Fix implementation of VRSQRTS

[Aurelien Jarno]

On Fri, Jan 07, 2011 at 09:26:25AM +0000, Peter Maydell wrote:
> The implementation of the ARM VRSQRTS instruction (which calculates
> (3 - op1 * op2) / 2) was missing the division operation. It also
> did not handle the special cases of (0,inf) and (inf,0).
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 705b99f..ac47de0 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2612,8 +2612,16 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env)
>  float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env)
>  {
>      float_status *s = &env->vfp.fp_status;
> +    float32 two = int32_to_float32(2, s);
>      float32 three = int32_to_float32(3, s);
> -    return float32_sub(three, float32_mul(a, b, s), s);
> +    float32 product;
> +    if ((float32_is_infinity(a) && float32_is_zero_or_denormal(b)) ||
> +        (float32_is_infinity(b) && float32_is_zero_or_denormal(a))) {
> +        product = float32_zero;
> +    } else {
> +        product = float32_mul(a, b, s);
> +    }
> +    return float32_div(float32_sub(three, product, s), two, s);
>  }
>  
>  /* NEON helpers.  */
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 3/4] target-arm: Add support for 'Standard FPSCR Value' as used by Neon

[Peter Maydell]

Add support to the ARM helper routines for a second fp_status value
which should be used for operations which the ARM ARM indicates use
"ARM standard floating-point arithmetic" rather than being controlled
by the rounding/flush/NaN settings in the FPSCR.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu.h    |   13 +++++++++++++
 target-arm/helper.c |    5 +++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 340933e..e501cf5 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -173,7 +173,20 @@ typedef struct CPUARMState {
         /* scratch space when Tn are not sufficient.  */
         uint32_t scratch[8];
 
+        /* fp_status is the "normal" fp status. standard_fp_status retains
+         * values corresponding to the ARM "Standard FPSCR Value", ie
+         * default-NaN, flush-to-zero, round-to-nearest and is used by
+         * any operations (generally Neon) which the architecture defines
+         * as controlled by the standard FPSCR value rather than the FPSCR.
+         *
+         * To avoid having to transfer exception bits around, we simply
+         * say that the FPSCR cumulative exception flags are the logical
+         * OR of the flags in the two fp statuses. This relies on the
+         * only thing which needs to read the exception flags being
+         * an explicit FPSCR read.
+         */
         float_status fp_status;
+        float_status standard_fp_status;
     } vfp;
     uint32_t exclusive_addr;
     uint32_t exclusive_val;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index ac47de0..d779055 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -237,6 +237,9 @@ void cpu_reset(CPUARMState *env)
     env->vfp.xregs[ARM_VFP_FPEXC] = 0;
     env->cp15.c2_base_mask = 0xffffc000u;
 #endif
+    set_flush_to_zero(1, &env->vfp.standard_fp_status);
+    set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);
+    set_default_nan_mode(1, &env->vfp.standard_fp_status);
     tlb_flush(env, 1);
 }
 
@@ -2256,6 +2259,7 @@ uint32_t HELPER(vfp_get_fpscr)(CPUState *env)
             | (env->vfp.vec_len << 16)
             | (env->vfp.vec_stride << 20);
     i = get_float_exception_flags(&env->vfp.fp_status);
+    i |= get_float_exception_flags(&env->vfp.standard_fp_status);
     fpscr |= vfp_exceptbits_from_host(i);
     return fpscr;
 }
@@ -2323,6 +2327,7 @@ void HELPER(vfp_set_fpscr)(CPUState *env, uint32_t val)
 
     i = vfp_exceptbits_to_host(val);
     set_float_exception_flags(i, &env->vfp.fp_status);
+    set_float_exception_flags(0, &env->vfp.standard_fp_status);
 }
 
 void vfp_set_fpscr(CPUState *env, uint32_t val)
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 3/4] target-arm: Add support for 'Standard FPSCR Value' as used by Neon

[Aurelien Jarno]

On Fri, Jan 07, 2011 at 09:26:26AM +0000, Peter Maydell wrote:
> Add support to the ARM helper routines for a second fp_status value
> which should be used for operations which the ARM ARM indicates use
> "ARM standard floating-point arithmetic" rather than being controlled
> by the rounding/flush/NaN settings in the FPSCR.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/cpu.h    |   13 +++++++++++++
>  target-arm/helper.c |    5 +++++
>  2 files changed, 18 insertions(+), 0 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 340933e..e501cf5 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -173,7 +173,20 @@ typedef struct CPUARMState {
>          /* scratch space when Tn are not sufficient.  */
>          uint32_t scratch[8];
>  
> +        /* fp_status is the "normal" fp status. standard_fp_status retains
> +         * values corresponding to the ARM "Standard FPSCR Value", ie
> +         * default-NaN, flush-to-zero, round-to-nearest and is used by
> +         * any operations (generally Neon) which the architecture defines
> +         * as controlled by the standard FPSCR value rather than the FPSCR.
> +         *
> +         * To avoid having to transfer exception bits around, we simply
> +         * say that the FPSCR cumulative exception flags are the logical
> +         * OR of the flags in the two fp statuses. This relies on the
> +         * only thing which needs to read the exception flags being
> +         * an explicit FPSCR read.
> +         */
>          float_status fp_status;
> +        float_status standard_fp_status;
>      } vfp;
>      uint32_t exclusive_addr;
>      uint32_t exclusive_val;
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index ac47de0..d779055 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -237,6 +237,9 @@ void cpu_reset(CPUARMState *env)
>      env->vfp.xregs[ARM_VFP_FPEXC] = 0;
>      env->cp15.c2_base_mask = 0xffffc000u;
>  #endif
> +    set_flush_to_zero(1, &env->vfp.standard_fp_status);
> +    set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);
> +    set_default_nan_mode(1, &env->vfp.standard_fp_status);
>      tlb_flush(env, 1);
>  }
>  
> @@ -2256,6 +2259,7 @@ uint32_t HELPER(vfp_get_fpscr)(CPUState *env)
>              | (env->vfp.vec_len << 16)
>              | (env->vfp.vec_stride << 20);
>      i = get_float_exception_flags(&env->vfp.fp_status);
> +    i |= get_float_exception_flags(&env->vfp.standard_fp_status);
>      fpscr |= vfp_exceptbits_from_host(i);
>      return fpscr;
>  }
> @@ -2323,6 +2327,7 @@ void HELPER(vfp_set_fpscr)(CPUState *env, uint32_t val)
>  
>      i = vfp_exceptbits_to_host(val);
>      set_float_exception_flags(i, &env->vfp.fp_status);
> +    set_float_exception_flags(0, &env->vfp.standard_fp_status);
>  }
>  
>  void vfp_set_fpscr(CPUState *env, uint32_t val)
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 4/4] target-arm: Use the standard FPSCR value for VRSQRTS

[Peter Maydell]

VSQRTS always uses the standard FPSCR value as it is a Neon instruction.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index d779055..8186500 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2616,7 +2616,7 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env)
 
 float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env)
 {
-    float_status *s = &env->vfp.fp_status;
+    float_status *s = &env->vfp.standard_fp_status;
     float32 two = int32_to_float32(2, s);
     float32 three = int32_to_float32(3, s);
     float32 product;
-- 
1.6.3.3

Re: [Qemu-devel] [PATCH 4/4] target-arm: Use the standard FPSCR value for VRSQRTS

[Aurelien Jarno]

On Fri, Jan 07, 2011 at 09:26:27AM +0000, Peter Maydell wrote:
> VSQRTS always uses the standard FPSCR value as it is a Neon instruction.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
 
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d779055..8186500 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2616,7 +2616,7 @@ float32 HELPER(recps_f32)(float32 a, float32 b, CPUState *env)
>  
>  float32 HELPER(rsqrts_f32)(float32 a, float32 b, CPUState *env)
>  {
> -    float_status *s = &env->vfp.fp_status;
> +    float_status *s = &env->vfp.standard_fp_status;
>      float32 two = int32_to_float32(2, s);
>      float32 three = int32_to_float32(3, s);
>      float32 product;
> -- 
> 1.6.3.3
> 
> 
> 

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/4] target-arm: Fix VRSQRTS

[Aurelien Jarno]

On Fri, Jan 07, 2011 at 09:26:23AM +0000, Peter Maydell wrote:
> This patch series fixes various bugs in VRSQRTS including the
> major one that it was calculating completely the wrong value
> (missing a division by 2).
> 
> It also introduces the infrastructure to support the ARM
> "Standard FPSCR Value" as used for operations which use "ARM
> standard floating-point arithmetic" rather than being controlled
> by the rounding/flush/NaN settings in the FPSCR. (Most Neon FP
> instructions should be using this, so there will be subsequent
> patches to use it with other instructions too.)
> 
> The "missing divide" bug was fixed in meego-qemu commit 438a549e0;
> the remaining fixes here are new.
> 
> Tested in the usual random-instruction-generation fashion.
> 
> Peter Maydell (4):
>   softfloat: Add float32_is_zero_or_denormal() function
>   target-arm: Fix implementation of VRSQRTS
>   target-arm: Add support for 'Standard FPSCR Value' as used by Neon
>   target-arm: Use the standard FPSCR value for VRSQRTS
> 
>  fpu/softfloat.h     |    5 +++++
>  target-arm/cpu.h    |   13 +++++++++++++
>  target-arm/helper.c |   17 +++++++++++++++--
>  3 files changed, 33 insertions(+), 2 deletions(-)
> 

Thanks, all applied.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net