diff mbox series

[v2,2/2] fpu/softfloat: raise float_invalid for NaN/Inf in round_to_int_and_pack

Message ID 20180413140334.26622-3-alex.bennee@linaro.org
State Accepted
Commit 801bc56336a127d9b351b3a2cc0336e4d0cb2686
Headers show
Series Softfloat Fixes for 2.12 | expand

Commit Message

Alex Bennée April 13, 2018, 2:03 p.m. UTC
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(+)

-- 
2.17.0

Comments

Bastian Koppelmann April 13, 2018, 2:06 p.m. UTC | #1
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
Peter Maydell April 13, 2018, 2:07 p.m. UTC | #2
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
Peter Maydell April 13, 2018, 2:07 p.m. UTC | #3
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
Bastian Koppelmann April 13, 2018, 3:24 p.m. UTC | #4
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
Peter Maydell April 13, 2018, 3:29 p.m. UTC | #5
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
Bastian Koppelmann April 13, 2018, 4:17 p.m. UTC | #6
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
Bastian Koppelmann April 16, 2018, 12:44 p.m. UTC | #7
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
diff mbox series

Patch

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;