[Qemu-devel] [PATCH v1 0/2] Softfloat Fixes for 2.12

[Qemu-devel] [PATCH v1 0/2] Softfloat Fixes for 2.12

[Alex Bennée]

Hi,

I'd hope to include fixes for our longstanding fcvt bugs but I ran out
of time. The fixes in this series are purely regression fixes from the
softfloat re-factor. On the plus side I have a very good test case of
fcvt now ;-)

Only my patch is not reviewed.

Alex Bennée (1):
  fpu/softfloat: raise float_invalid for NaN in float_to_int

Emilio G. Cota (1):
  softfloat: fix {min,max}nummag for same-abs-value inputs

 fpu/softfloat.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

-- 
2.17.0

[Qemu-devel] [PATCH v1 1/2] softfloat: fix {min, max}nummag for same-abs-value inputs

[Alex Bennée]

From: "Emilio G. Cota" <cota@braap.org>

Before 8936006 ("fpu/softfloat: re-factor minmax", 2018-02-21),
we used to return +Zero for maxnummag(-Zero,+Zero); after that
commit, we return -Zero.

Fix it by making {min,max}nummag consistent with {min,max}num,
deferring to the latter when the absolute value of the operands
is the same.

With this fix we now pass fp-test.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Emilio G. Cota <cota@braap.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 fpu/softfloat.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index b46dccc63e..9b99aa6ec8 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1704,7 +1704,6 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
         return pick_nan(a, b, s);
     } else {
         int a_exp, b_exp;
-        bool a_sign, b_sign;
 
         switch (a.cls) {
         case float_class_normal:
@@ -1735,20 +1734,22 @@ static FloatParts minmax_floats(FloatParts a, FloatParts b, bool ismin,
             break;
         }
 
-        a_sign = a.sign;
-        b_sign = b.sign;
-        if (ismag) {
-            a_sign = b_sign = 0;
+        if (ismag && (a_exp != b_exp || a.frac != b.frac)) {
+            bool a_less = a_exp < b_exp;
+            if (a_exp == b_exp) {
+                a_less = a.frac < b.frac;
+            }
+            return a_less ^ ismin ? b : a;
         }
 
-        if (a_sign == b_sign) {
+        if (a.sign == b.sign) {
             bool a_less = a_exp < b_exp;
             if (a_exp == b_exp) {
                 a_less = a.frac < b.frac;
             }
-            return a_sign ^ a_less ^ ismin ? b : a;
+            return a.sign ^ a_less ^ ismin ? b : a;
         } else {
-            return a_sign ^ ismin ? b : a;
+            return a.sign ^ ismin ? b : a;
         }
     }
 }
-- 
2.17.0

[Qemu-devel] [PATCH v1 2/2] fpu/softfloat: raise float_invalid for NaN in float_to_int

[Alex Bennée]

Fixes https://bugs.launchpad.net/qemu/+bug/1759264

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
---
 fpu/softfloat.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 9b99aa6ec8..ddc77c273c 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1344,6 +1344,7 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
     case float_class_qnan:
     case float_class_dnan:
     case float_class_msnan:
+        s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
     case float_class_inf:
         return p.sign ? min : max;
-- 
2.17.0

Re: [Qemu-devel] [PATCH v1 2/2] fpu/softfloat: raise float_invalid for NaN in float_to_int

[Peter Maydell]

On 12 April 2018 at 12:58, Alex Bennée <alex.bennee@linaro.org> wrote:
> Fixes https://bugs.launchpad.net/qemu/+bug/1759264
>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> ---
>  fpu/softfloat.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 9b99aa6ec8..ddc77c273c 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1344,6 +1344,7 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
>      case float_class_qnan:
>      case float_class_dnan:
>      case float_class_msnan:
> +        s->float_exception_flags = orig_flags | float_flag_invalid;
>          return max;
>      case float_class_inf:
>          return p.sign ? min : max;

Don't we also need to raise the Invalid flag for float_class_inf ?

In both cases, this is fixing a regression introduced in
commit ab52f973a50, I think.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] fpu/softfloat: raise float_invalid for NaN in float_to_int

[Bastian Koppelmann]

On 04/12/2018 02:26 PM, Peter Maydell wrote:
> On 12 April 2018 at 12:58, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Fixes https://bugs.launchpad.net/qemu/+bug/1759264
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> ---
>>  fpu/softfloat.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 9b99aa6ec8..ddc77c273c 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -1344,6 +1344,7 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
>>      case float_class_qnan:
>>      case float_class_dnan:
>>      case float_class_msnan:
>> +        s->float_exception_flags = orig_flags | float_flag_invalid;
>>          return max;
>>      case float_class_inf:
>>          return p.sign ? min : max;
> 
> Don't we also need to raise the Invalid flag for float_class_inf ?
> 

Yup, as mentioned in the bug report.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH v1 2/2] fpu/softfloat: raise float_invalid for NaN in float_to_int

[Alex Bennée]

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

> On 12 April 2018 at 12:58, Alex Bennée <alex.bennee@linaro.org> wrote:
>> Fixes https://bugs.launchpad.net/qemu/+bug/1759264
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
>> ---
>>  fpu/softfloat.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index 9b99aa6ec8..ddc77c273c 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -1344,6 +1344,7 @@ static int64_t round_to_int_and_pack(FloatParts in, int rmode,
>>      case float_class_qnan:
>>      case float_class_dnan:
>>      case float_class_msnan:
>> +        s->float_exception_flags = orig_flags | float_flag_invalid;
>>          return max;
>>      case float_class_inf:
>>          return p.sign ? min : max;
>
> Don't we also need to raise the Invalid flag for float_class_inf ?
>
> In both cases, this is fixing a regression introduced in
> commit ab52f973a50, I think.

And I guess the round_to_uint_and_pack as well which doesn't handle the
input inf case.

>
> thanks
> -- PMM


--
Alex Bennée