diff mbox

[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 (mailing list archive)
State New, archived
Headers show

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(+)

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

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;