[Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes

[Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes

[Richard Henderson]

An alternate fix for Bastian's division problem and another
fix for a scalbn problem exposed during SVE testing.  Again,
the SVE issue should be visible with normal NEON insns too.


r~


Richard Henderson (2):
  fpu: Check for inf/x before x/0
  fpu: Bound increment for scalbn

 fpu/softfloat.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0

[Richard Henderson]

The re-factoring of div_floats changed the order of checking meaning
an operation like -inf/0 erroneously raises the divbyzero flag.
IEEE-754 (2008) specifies this should only occur for operations
on finite operands.

We fix this by moving the check on the dividend being Inf/0 to
before the divisor is zero check.

Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index fb8663f59e..ba6e654050 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1146,15 +1146,20 @@ static FloatParts div_floats(FloatParts a, FloatParts b, float_status *s)
         a.cls = float_class_dnan;
         return a;
     }
-    /* Div 0 => Inf */
+    /* Inf / x */
+    if (a.cls == float_class_inf) {
+        a.sign = sign;
+        return a;
+    }
+    /* x / 0 => Inf */
     if (b.cls == float_class_zero) {
         s->float_exception_flags |= float_flag_divbyzero;
         a.cls = float_class_inf;
         a.sign = sign;
         return a;
     }
-    /* Inf / x or 0 / x */
-    if (a.cls == float_class_inf || a.cls == float_class_zero) {
+    /* 0 / x */
+    if (a.cls == float_class_zero) {
         a.sign = sign;
         return a;
     }
-- 
2.14.3

Re: [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0

[Peter Maydell]

On 17 April 2018 at 03:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
> The re-factoring of div_floats changed the order of checking meaning
> an operation like -inf/0 erroneously raises the divbyzero flag.
> IEEE-754 (2008) specifies this should only occur for operations
> on finite operands.
>
> We fix this by moving the check on the dividend being Inf/0 to
> before the divisor is zero check.
>

I've applied Alex's fix to master for this, since it seems
more straightforward.

thanks
-- PMM

[Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn

[Richard Henderson]

Without bounding the increment, we can overflow exp either here
in scalbn_decomposed or when adding the bias in round_canonical.
This can result in e.g. underflowing to 0 instead of overflowing
to infinity.

The old softfloat code did bound the increment.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 fpu/softfloat.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index ba6e654050..a589f328c9 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
         return return_nan(a, s);
     }
     if (a.cls == float_class_normal) {
+        /* The largest float type (even though not supported by FloatParts)
+         * is float128, which has a 15 bit exponent.  Bounding N to 16 bits
+         * still allows rounding to infinity, without allowing overflow
+         * within the int32_t that backs FloatParts.exp.
+         */
+        n = MIN(MAX(n, -0x10000), 0x10000);
         a.exp += n;
     }
     return a;
-- 
2.14.3

Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn

[Peter Maydell]

On 17 April 2018 at 03:53, Richard Henderson
<richard.henderson@linaro.org> wrote:
> Without bounding the increment, we can overflow exp either here
> in scalbn_decomposed or when adding the bias in round_canonical.
> This can result in e.g. underflowing to 0 instead of overflowing
> to infinity.
>
> The old softfloat code did bound the increment.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index ba6e654050..a589f328c9 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
>          return return_nan(a, s);
>      }
>      if (a.cls == float_class_normal) {
> +        /* The largest float type (even though not supported by FloatParts)
> +         * is float128, which has a 15 bit exponent.  Bounding N to 16 bits
> +         * still allows rounding to infinity, without allowing overflow
> +         * within the int32_t that backs FloatParts.exp.
> +         */
> +        n = MIN(MAX(n, -0x10000), 0x10000);
>          a.exp += n;
>      }
>      return a;
> --

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

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn

[Alex Bennée]

Richard Henderson <richard.henderson@linaro.org> writes:

> Without bounding the increment, we can overflow exp either here
> in scalbn_decomposed or when adding the bias in round_canonical.
> This can result in e.g. underflowing to 0 instead of overflowing
> to infinity.
>
> The old softfloat code did bound the increment.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index ba6e654050..a589f328c9 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
>          return return_nan(a, s);
>      }
>      if (a.cls == float_class_normal) {
> +        /* The largest float type (even though not supported by FloatParts)
> +         * is float128, which has a 15 bit exponent.  Bounding N to 16 bits
> +         * still allows rounding to infinity, without allowing overflow
> +         * within the int32_t that backs FloatParts.exp.
> +         */
> +        n = MIN(MAX(n, -0x10000), 0x10000);
>          a.exp += n;
>      }
>      return a;

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

(risu FWIW although it obviously didn't catch this failure ;-)

--
Alex Bennée

Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn

[Peter Maydell]

On 17 April 2018 at 14:51, Alex Bennée <alex.bennee@linaro.org> wrote:
>
> Richard Henderson <richard.henderson@linaro.org> writes:
>
>> Without bounding the increment, we can overflow exp either here
>> in scalbn_decomposed or when adding the bias in round_canonical.
>> This can result in e.g. underflowing to 0 instead of overflowing
>> to infinity.
>>
>> The old softfloat code did bound the increment.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  fpu/softfloat.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
>> index ba6e654050..a589f328c9 100644
>> --- a/fpu/softfloat.c
>> +++ b/fpu/softfloat.c
>> @@ -1883,6 +1883,12 @@ static FloatParts scalbn_decomposed(FloatParts a, int n, float_status *s)
>>          return return_nan(a, s);
>>      }
>>      if (a.cls == float_class_normal) {
>> +        /* The largest float type (even though not supported by FloatParts)
>> +         * is float128, which has a 15 bit exponent.  Bounding N to 16 bits
>> +         * still allows rounding to infinity, without allowing overflow
>> +         * within the int32_t that backs FloatParts.exp.
>> +         */
>> +        n = MIN(MAX(n, -0x10000), 0x10000);
>>          a.exp += n;
>>      }
>>      return a;
>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>
> (risu FWIW although it obviously didn't catch this failure ;-)

Thanks; applied this patch to master.

-- PMM