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

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

[Alex Bennée]

Hi,

The float_invalid patch now handles the Inf case as well and includes
the fix to round_to_uint_and_pack.

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

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

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

-- 
2.17.0

[Qemu-devel] [PATCH v2 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 v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Alex Bennée]

The re-factor broke the raising of INVALID when NaN/Inf is passed to
the float_to_int conversion functions. round_to_uint_and_pack got this
right for NaN but also missed out the Inf handling.

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 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 9b99aa6ec8..fb8663f59e 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1344,8 +1344,10 @@ 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:
+        s->float_exception_flags = orig_flags | float_flag_invalid;
         return p.sign ? min : max;
     case float_class_zero:
         return 0;
@@ -1437,6 +1439,7 @@ static uint64_t round_to_uint_and_pack(FloatParts in, int rmode, uint64_t max,
         s->float_exception_flags = orig_flags | float_flag_invalid;
         return max;
     case float_class_inf:
+        s->float_exception_flags = orig_flags | float_flag_invalid;
         return p.sign ? 0 : max;
     case float_class_zero:
         return 0;
-- 
2.17.0

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Bastian Koppelmann]

On 04/13/2018 04:03 PM, Alex Bennée wrote:
> The re-factor broke the raising of INVALID when NaN/Inf is passed to
> the float_to_int conversion functions. round_to_uint_and_pack got this
> right for NaN but also missed out the Inf handling.
> 
> 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 | 3 +++
>  1 file changed, 3 insertions(+)
> 

Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Cheers,
Bastian

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Peter Maydell]

On 13 April 2018 at 15:06, Bastian Koppelmann
<kbastian@mail.uni-paderborn.de> wrote:
> On 04/13/2018 04:03 PM, Alex Bennée wrote:
>> The re-factor broke the raising of INVALID when NaN/Inf is passed to
>> the float_to_int conversion functions. round_to_uint_and_pack got this
>> right for NaN but also missed out the Inf handling.
>>
>> 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 | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>
> Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

Could you run your tricore test case with this patch? I think
Alex was having difficulty getting that running...

thanks
--  PMM

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Bastian Koppelmann]

On 04/13/2018 04:07 PM, Peter Maydell wrote:
> On 13 April 2018 at 15:06, Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de> wrote:
>> On 04/13/2018 04:03 PM, Alex Bennée wrote:
>>> The re-factor broke the raising of INVALID when NaN/Inf is passed to
>>> the float_to_int conversion functions. round_to_uint_and_pack got this
>>> right for NaN but also missed out the Inf handling.
>>>
>>> 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 | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>
>> Reviewed-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> 
> Could you run your tricore test case with this patch? I think
> Alex was having difficulty getting that running...

This fixes the ftoi problem, so
Tested-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

However, my risu-like tests found another flag raising problem with
float32_div. I'll investigate it on Monday.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Peter Maydell]

On 13 April 2018 at 16:24, Bastian Koppelmann
<kbastian@mail.uni-paderborn.de> wrote:
> However, my risu-like tests found another flag raising problem with
> float32_div. I'll investigate it on Monday.

Is it a regression from our previous (2.11) behaviour?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Bastian Koppelmann]

On 04/13/2018 05:29 PM, Peter Maydell wrote:
> On 13 April 2018 at 16:24, Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de> wrote:
>> However, my risu-like tests found another flag raising problem with
>> float32_div. I'll investigate it on Monday.
> 
> Is it a regression from our previous (2.11) behaviour?

Yes, for tag v2.11.0 my failing test succeeds.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Bastian Koppelmann]

On 04/13/2018 05:29 PM, Peter Maydell wrote:
> On 13 April 2018 at 16:24, Bastian Koppelmann
> <kbastian@mail.uni-paderborn.de> wrote:
>> However, my risu-like tests found another flag raising problem with
>> float32_div. I'll investigate it on Monday.

I found the problem. With float32_div(a, b), if a is inf and b is not inf we
just returned a without raising any flag. In div_floats() after the refactor we
don't have this case.

In my testcase I divided -inf by 0 so the div_floats() function ran into
div_by_zero and wrongly raised the corresponding flag.

Cheers,
Bastian

Re: [Qemu-devel] [PATCH v2 2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

[Peter Maydell]

On 13 April 2018 at 15:03, Alex Bennée <alex.bennee@linaro.org> wrote:
> The re-factor broke the raising of INVALID when NaN/Inf is passed to
> the float_to_int conversion functions. round_to_uint_and_pack got this
> right for NaN but also missed out the Inf handling.
>
> 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>
> ---

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

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/2] Softfloat Fixes for 2.12

[Peter Maydell]

On 13 April 2018 at 15:03, Alex Bennée <alex.bennee@linaro.org> wrote:
> Hi,
>
> The float_invalid patch now handles the Inf case as well and includes
> the fix to round_to_uint_and_pack.
>
> Alex Bennée (1):
>   fpu/softfloat: raise float_invalid for NaN/Inf in
>     round_to_int_and_pack
>
> Emilio G. Cota (1):
>   softfloat: fix {min,max}nummag for same-abs-value inputs
>
>  fpu/softfloat.c | 20 ++++++++++++--------
>  1 file changed, 12 insertions(+), 8 deletions(-)

Applied to master for 2.12, thanks.

-- PMM