qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range
@ 2013-12-19 22:00 Peter Maydell
  2013-12-19 22:11 ` Peter Maydell
  2013-12-22 18:03 ` Aurelien Jarno
  0 siblings, 2 replies; 3+ messages in thread
From: Peter Maydell @ 2013-12-19 22:00 UTC (permalink / raw)
  To: qemu-devel; +Cc: Tom Musta, patches

We implement a number of float-to-integer conversions using conversion
to an integer type with a wider range and then a check against the
narrower range we are actually converting to. If we find the result to
be out of range we correctly raise the Invalid exception, but we must
also suppress other exceptions which might have been raised by the
conversion function we called.

This won't throw away exceptions we should have preserved, because for
the 'core' exception flags the IEEE spec mandates that the only valid
combinations of exception that can be raised by a single operation are
Inexact + Overflow and Inexact + Underflow. For the non-IEEE softfloat
flag for input denormals, we can guarantee that that flag won't have
been set for out of range float-to-int conversions because a squashed
denormal by definition goes to plus or minus zero, which is always in
range after conversion to integer zero.

This bug has been fixed for some of the float-to-int conversion routines
by previous patches; fix it for the remaining functions as well, so
that they all restore the pre-conversion status flags prior to raising
Invalid.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
NB that I've worded the commit message on the assumption that the
patches Tom Musta has written for PPC bugs go in first; as it happens
the patches don't actually conflict, though.

Some of these fix wrong-exception-flags bugs in 32 bit ARM VCVT.

 fpu/softfloat.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index dbda61b..253e6b3 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -6432,17 +6432,18 @@ uint32 float32_to_uint32( float32 a STATUS_PARAM )
 {
     int64_t v;
     uint32 res;
+    int old_exc_flags = get_float_exception_flags(status);
 
     v = float32_to_int64(a STATUS_VAR);
     if (v < 0) {
         res = 0;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else if (v > 0xffffffff) {
         res = 0xffffffff;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else {
-        res = v;
+        return v;
     }
+    set_float_exception_flags(old_exc_flags, status);
+    float_raise(float_flag_invalid STATUS_VAR);
     return res;
 }
 
@@ -6450,17 +6451,18 @@ uint32 float32_to_uint32_round_to_zero( float32 a STATUS_PARAM )
 {
     int64_t v;
     uint32 res;
+    int old_exc_flags = get_float_exception_flags(status);
 
     v = float32_to_int64_round_to_zero(a STATUS_VAR);
     if (v < 0) {
         res = 0;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else if (v > 0xffffffff) {
         res = 0xffffffff;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else {
-        res = v;
+        return v;
     }
+    set_float_exception_flags(old_exc_flags, status);
+    float_raise(float_flag_invalid STATUS_VAR);
     return res;
 }
 
@@ -6468,17 +6470,18 @@ uint_fast16_t float32_to_uint16_round_to_zero(float32 a STATUS_PARAM)
 {
     int64_t v;
     uint_fast16_t res;
+    int old_exc_flags = get_float_exception_flags(status);
 
     v = float32_to_int64_round_to_zero(a STATUS_VAR);
     if (v < 0) {
         res = 0;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else if (v > 0xffff) {
         res = 0xffff;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else {
-        res = v;
+        return v;
     }
+    set_float_exception_flags(old_exc_flags, status);
+    float_raise(float_flag_invalid STATUS_VAR);
     return res;
 }
 
@@ -6522,17 +6525,18 @@ uint_fast16_t float64_to_uint16_round_to_zero(float64 a STATUS_PARAM)
 {
     int64_t v;
     uint_fast16_t res;
+    int old_exc_flags = get_float_exception_flags(status);
 
     v = float64_to_int64_round_to_zero(a STATUS_VAR);
     if (v < 0) {
         res = 0;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else if (v > 0xffff) {
         res = 0xffff;
-        float_raise( float_flag_invalid STATUS_VAR);
     } else {
-        res = v;
+        return v;
     }
+    set_float_exception_flags(old_exc_flags, status);
+    float_raise(float_flag_invalid STATUS_VAR);
     return res;
 }
 
-- 
1.8.5

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range
  2013-12-19 22:00 [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range Peter Maydell
@ 2013-12-19 22:11 ` Peter Maydell
  2013-12-22 18:03 ` Aurelien Jarno
  1 sibling, 0 replies; 3+ messages in thread
From: Peter Maydell @ 2013-12-19 22:11 UTC (permalink / raw)
  To: QEMU Developers; +Cc: Tom Musta, Patch Tracking

On 19 December 2013 22:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> We implement a number of float-to-integer conversions using conversion
> to an integer type with a wider range and then a check against the
> narrower range we are actually converting to. If we find the result to
> be out of range we correctly raise the Invalid exception, but we must
> also suppress other exceptions which might have been raised by the
> conversion function we called.
>
> This won't throw away exceptions we should have preserved, because for
> the 'core' exception flags the IEEE spec mandates that the only valid
> combinations of exception that can be raised by a single operation are
> Inexact + Overflow and Inexact + Underflow. For the non-IEEE softfloat
> flag for input denormals, we can guarantee that that flag won't have
> been set for out of range float-to-int conversions because a squashed
> denormal by definition goes to plus or minus zero, which is always in
> range after conversion to integer zero.
>
> This bug has been fixed for some of the float-to-int conversion routines
> by previous patches; fix it for the remaining functions as well, so
> that they all restore the pre-conversion status flags prior to raising
> Invalid.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB that I've worded the commit message on the assumption that the
> patches Tom Musta has written for PPC bugs go in first; as it happens
> the patches don't actually conflict, though.
>
> Some of these fix wrong-exception-flags bugs in 32 bit ARM VCVT.

Forgot it for this one, so:
This patch is licensed under softfloat-2a or -2b, at your option.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range
  2013-12-19 22:00 [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range Peter Maydell
  2013-12-19 22:11 ` Peter Maydell
@ 2013-12-22 18:03 ` Aurelien Jarno
  1 sibling, 0 replies; 3+ messages in thread
From: Aurelien Jarno @ 2013-12-22 18:03 UTC (permalink / raw)
  To: Peter Maydell; +Cc: Tom Musta, qemu-devel, patches

On Thu, Dec 19, 2013 at 10:00:18PM +0000, Peter Maydell wrote:
> We implement a number of float-to-integer conversions using conversion
> to an integer type with a wider range and then a check against the
> narrower range we are actually converting to. If we find the result to
> be out of range we correctly raise the Invalid exception, but we must
> also suppress other exceptions which might have been raised by the
> conversion function we called.
> 
> This won't throw away exceptions we should have preserved, because for
> the 'core' exception flags the IEEE spec mandates that the only valid
> combinations of exception that can be raised by a single operation are
> Inexact + Overflow and Inexact + Underflow. For the non-IEEE softfloat
> flag for input denormals, we can guarantee that that flag won't have
> been set for out of range float-to-int conversions because a squashed
> denormal by definition goes to plus or minus zero, which is always in
> range after conversion to integer zero.
> 
> This bug has been fixed for some of the float-to-int conversion routines
> by previous patches; fix it for the remaining functions as well, so
> that they all restore the pre-conversion status flags prior to raising
> Invalid.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> NB that I've worded the commit message on the assumption that the
> patches Tom Musta has written for PPC bugs go in first; as it happens
> the patches don't actually conflict, though.
> 
> Some of these fix wrong-exception-flags bugs in 32 bit ARM VCVT.
> 
>  fpu/softfloat.c | 28 ++++++++++++++++------------
>  1 file changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index dbda61b..253e6b3 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -6432,17 +6432,18 @@ uint32 float32_to_uint32( float32 a STATUS_PARAM )
>  {
>      int64_t v;
>      uint32 res;
> +    int old_exc_flags = get_float_exception_flags(status);
>  
>      v = float32_to_int64(a STATUS_VAR);
>      if (v < 0) {
>          res = 0;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else if (v > 0xffffffff) {
>          res = 0xffffffff;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else {
> -        res = v;
> +        return v;
>      }
> +    set_float_exception_flags(old_exc_flags, status);
> +    float_raise(float_flag_invalid STATUS_VAR);
>      return res;
>  }
>  
> @@ -6450,17 +6451,18 @@ uint32 float32_to_uint32_round_to_zero( float32 a STATUS_PARAM )
>  {
>      int64_t v;
>      uint32 res;
> +    int old_exc_flags = get_float_exception_flags(status);
>  
>      v = float32_to_int64_round_to_zero(a STATUS_VAR);
>      if (v < 0) {
>          res = 0;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else if (v > 0xffffffff) {
>          res = 0xffffffff;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else {
> -        res = v;
> +        return v;
>      }
> +    set_float_exception_flags(old_exc_flags, status);
> +    float_raise(float_flag_invalid STATUS_VAR);
>      return res;
>  }
>  
> @@ -6468,17 +6470,18 @@ uint_fast16_t float32_to_uint16_round_to_zero(float32 a STATUS_PARAM)
>  {
>      int64_t v;
>      uint_fast16_t res;
> +    int old_exc_flags = get_float_exception_flags(status);
>  
>      v = float32_to_int64_round_to_zero(a STATUS_VAR);
>      if (v < 0) {
>          res = 0;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else if (v > 0xffff) {
>          res = 0xffff;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else {
> -        res = v;
> +        return v;
>      }
> +    set_float_exception_flags(old_exc_flags, status);
> +    float_raise(float_flag_invalid STATUS_VAR);
>      return res;
>  }
>  
> @@ -6522,17 +6525,18 @@ uint_fast16_t float64_to_uint16_round_to_zero(float64 a STATUS_PARAM)
>  {
>      int64_t v;
>      uint_fast16_t res;
> +    int old_exc_flags = get_float_exception_flags(status);
>  
>      v = float64_to_int64_round_to_zero(a STATUS_VAR);
>      if (v < 0) {
>          res = 0;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else if (v > 0xffff) {
>          res = 0xffff;
> -        float_raise( float_flag_invalid STATUS_VAR);
>      } else {
> -        res = v;
> +        return v;
>      }
> +    set_float_exception_flags(old_exc_flags, status);
> +    float_raise(float_flag_invalid STATUS_VAR);
>      return res;
>  }
>  

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2013-12-22 18:03 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-19 22:00 [Qemu-devel] [PATCH] softfloat: Only raise Invalid when conversions to int are out of range Peter Maydell
2013-12-19 22:11 ` Peter Maydell
2013-12-22 18:03 ` Aurelien Jarno

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).