* [Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes
@ 2018-04-17 2:53 Richard Henderson
2018-04-17 2:53 ` [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0 Richard Henderson
2018-04-17 2:53 ` [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn Richard Henderson
0 siblings, 2 replies; 7+ messages in thread
From: Richard Henderson @ 2018-04-17 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0
2018-04-17 2:53 [Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes Richard Henderson
@ 2018-04-17 2:53 ` Richard Henderson
2018-04-17 9:02 ` Peter Maydell
2018-04-17 2:53 ` [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn Richard Henderson
1 sibling, 1 reply; 7+ messages in thread
From: Richard Henderson @ 2018-04-17 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee, Bastian Koppelmann
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn
2018-04-17 2:53 [Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes Richard Henderson
2018-04-17 2:53 ` [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0 Richard Henderson
@ 2018-04-17 2:53 ` Richard Henderson
2018-04-17 9:53 ` Peter Maydell
2018-04-17 13:51 ` Alex Bennée
1 sibling, 2 replies; 7+ messages in thread
From: Richard Henderson @ 2018-04-17 2:53 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, alex.bennee
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0
2018-04-17 2:53 ` [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0 Richard Henderson
@ 2018-04-17 9:02 ` Peter Maydell
0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2018-04-17 9:02 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, Alex Bennée, Bastian Koppelmann
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn
2018-04-17 2:53 ` [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn Richard Henderson
@ 2018-04-17 9:53 ` Peter Maydell
2018-04-17 13:51 ` Alex Bennée
1 sibling, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2018-04-17 9:53 UTC (permalink / raw)
To: Richard Henderson; +Cc: QEMU Developers, Alex Bennée
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn
2018-04-17 2:53 ` [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn Richard Henderson
2018-04-17 9:53 ` Peter Maydell
@ 2018-04-17 13:51 ` Alex Bennée
2018-04-17 13:53 ` Peter Maydell
1 sibling, 1 reply; 7+ messages in thread
From: Alex Bennée @ 2018-04-17 13:51 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, peter.maydell
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn
2018-04-17 13:51 ` Alex Bennée
@ 2018-04-17 13:53 ` Peter Maydell
0 siblings, 0 replies; 7+ messages in thread
From: Peter Maydell @ 2018-04-17 13:53 UTC (permalink / raw)
To: Alex Bennée; +Cc: Richard Henderson, QEMU Developers
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-04-17 13:53 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-17 2:53 [Qemu-devel] [PATCH for-2.12 0/2] softfloat fixes Richard Henderson
2018-04-17 2:53 ` [Qemu-devel] [PATCH 1/2] fpu: Check for inf/x before x/0 Richard Henderson
2018-04-17 9:02 ` Peter Maydell
2018-04-17 2:53 ` [Qemu-devel] [PATCH 2/2] fpu: Bound increment for scalbn Richard Henderson
2018-04-17 9:53 ` Peter Maydell
2018-04-17 13:51 ` Alex Bennée
2018-04-17 13:53 ` Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.