[v2,02/27] fpu/softfloat: Don't set Invalid for float-to-int(MAXINT)

Message ID 20180512004311.9299-3-richard.henderson@linaro.org
State New
Headers show
Series
  • softfloat patch roundup
Related show

Commit Message

Richard Henderson May 12, 2018, 12:42 a.m.
From: Peter Maydell <peter.maydell@linaro.org>


In float-to-integer conversion, if the floating point input
converts exactly to the largest or smallest integer that
fits in to the result type, this is not an overflow.
In this situation we were producing the correct result value,
but were incorrectly setting the Invalid flag.
For example for Arm A64, "FCVTAS w0, d0" on an input of
0x41dfffffffc00000 should produce 0x7fffffff and set no flags.

Fix the boundary case to take the right half of the if()
statements.

This fixes a regression from 2.11 introduced by the softfloat
refactoring.

Cc: qemu-stable@nongnu.org
Fixes: ab52f973a50
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 fpu/softfloat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.17.0

Comments

Peter Maydell May 14, 2018, 10:19 a.m. | #1
On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:
> From: Peter Maydell <peter.maydell@linaro.org>

>

> In float-to-integer conversion, if the floating point input

> converts exactly to the largest or smallest integer that

> fits in to the result type, this is not an overflow.

> In this situation we were producing the correct result value,

> but were incorrectly setting the Invalid flag.

> For example for Arm A64, "FCVTAS w0, d0" on an input of

> 0x41dfffffffc00000 should produce 0x7fffffff and set no flags.

>

> Fix the boundary case to take the right half of the if()

> statements.

>

> This fixes a regression from 2.11 introduced by the softfloat

> refactoring.

>

> Cc: qemu-stable@nongnu.org

> Fixes: ab52f973a50

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


This is missing your Signed-off-by: line (and a reviewed-by
would be nice too :-))

> ---

>  fpu/softfloat.c | 4 ++--

>  1 file changed, 2 insertions(+), 2 deletions(-)

>

> diff --git a/fpu/softfloat.c b/fpu/softfloat.c

> index b39c0c6fbb..bc0f52fa54 100644

> --- a/fpu/softfloat.c

> +++ b/fpu/softfloat.c

> @@ -1368,14 +1368,14 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,

>              r = UINT64_MAX;

>          }

>          if (p.sign) {

> -            if (r < -(uint64_t) min) {

> +            if (r <= -(uint64_t) min) {

>                  return -r;

>              } else {

>                  s->float_exception_flags = orig_flags | float_flag_invalid;

>                  return min;

>              }

>          } else {

> -            if (r < max) {

> +            if (r <= max) {

>                  return r;

>              } else {

>                  s->float_exception_flags = orig_flags | float_flag_invalid;

> --

> 2.17.0


thanks
-- PMM
Richard Henderson May 14, 2018, 4:16 p.m. | #2
On 05/14/2018 03:19 AM, Peter Maydell wrote:
> On 12 May 2018 at 01:42, Richard Henderson <richard.henderson@linaro.org> wrote:

>> From: Peter Maydell <peter.maydell@linaro.org>

>>

>> In float-to-integer conversion, if the floating point input

>> converts exactly to the largest or smallest integer that

>> fits in to the result type, this is not an overflow.

>> In this situation we were producing the correct result value,

>> but were incorrectly setting the Invalid flag.

>> For example for Arm A64, "FCVTAS w0, d0" on an input of

>> 0x41dfffffffc00000 should produce 0x7fffffff and set no flags.

>>

>> Fix the boundary case to take the right half of the if()

>> statements.

>>

>> This fixes a regression from 2.11 introduced by the softfloat

>> refactoring.

>>

>> Cc: qemu-stable@nongnu.org

>> Fixes: ab52f973a50

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> 

> This is missing your Signed-off-by: line (and a reviewed-by

> would be nice too :-))


Dang it.  I thought I had done so for all of the patches.
Clearly missed one though.  For your target-arm.next branch:

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



r~

Patch

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index b39c0c6fbb..bc0f52fa54 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1368,14 +1368,14 @@  static int64_t round_to_int_and_pack(FloatParts in, int rmode,
             r = UINT64_MAX;
         }
         if (p.sign) {
-            if (r < -(uint64_t) min) {
+            if (r <= -(uint64_t) min) {
                 return -r;
             } else {
                 s->float_exception_flags = orig_flags | float_flag_invalid;
                 return min;
             }
         } else {
-            if (r < max) {
+            if (r <= max) {
                 return r;
             } else {
                 s->float_exception_flags = orig_flags | float_flag_invalid;