[PATCH v4 11/12] fpu: Saturate the exponent in uncanon_normal

Richard Henderson posted 12 patches 2 months ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Peter Maydell <peter.maydell@linaro.org>, "Alex Bennée" <alex.bennee@linaro.org>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>
There is a newer version of this series
[PATCH v4 11/12] fpu: Saturate the exponent in uncanon_normal
Posted by Richard Henderson 2 months ago
Notice the conflict between saturation and rebias_overflow,
which should never happen.  Otherwise, saturate to INT32_MAX
and allow the usual overflow to max, infinity, or dnan.

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

diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
index 45606f8402..914e588ec8 100644
--- a/fpu/softfloat-parts.c.inc
+++ b/fpu/softfloat-parts.c.inc
@@ -341,7 +341,17 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s,
         g_assert_not_reached();
     }
 
-    exp = p->exp + fmt->exp_bias;
+    /* Because exp_bias is positive, we can only overflow past INT_MAX. */
+    if (sadd32_overflow(p->exp, fmt->exp_bias, &exp)) {
+        /*
+         * rebias_overflow wants to compute a modulo exponent, which
+         * conflicts with saturation.  That said, saturation can only
+         * happen with scalbn, which is not a PowerPC operation.
+         */
+        assert(!s->rebias_overflow);
+        exp = INT32_MAX;
+    }
+
     if (likely(exp > 0)) {
         if (p->frac_lo & round_mask) {
             flags |= float_flag_inexact;
-- 
2.43.0
Re: [PATCH v4 11/12] fpu: Saturate the exponent in uncanon_normal
Posted by Peter Maydell 1 month, 4 weeks ago
On Sun, 17 May 2026 at 01:27, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Notice the conflict between saturation and rebias_overflow,
> which should never happen.  Otherwise, saturate to INT32_MAX
> and allow the usual overflow to max, infinity, or dnan.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat-parts.c.inc | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index 45606f8402..914e588ec8 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -341,7 +341,17 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s,
>          g_assert_not_reached();
>      }
>
> -    exp = p->exp + fmt->exp_bias;
> +    /* Because exp_bias is positive, we can only overflow past INT_MAX. */
> +    if (sadd32_overflow(p->exp, fmt->exp_bias, &exp)) {
> +        /*
> +         * rebias_overflow wants to compute a modulo exponent, which
> +         * conflicts with saturation.  That said, saturation can only
> +         * happen with scalbn, which is not a PowerPC operation.
> +         */
> +        assert(!s->rebias_overflow);
> +        exp = INT32_MAX;

As mentioned in the review comment on one of the Arm patches,
I think that if we set exp to INT32_MAX here then the
"round up" code below can increment it, overflowing to INT_MIN,
and then we won't correctly spot that it is too large for the format.

> +    }

thanks
-- PMM
Re: [PATCH v4 11/12] fpu: Saturate the exponent in uncanon_normal
Posted by Richard Henderson 1 month, 4 weeks ago
On 5/19/26 02:57, Peter Maydell wrote:
> On Sun, 17 May 2026 at 01:27, Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> Notice the conflict between saturation and rebias_overflow,
>> which should never happen.  Otherwise, saturate to INT32_MAX
>> and allow the usual overflow to max, infinity, or dnan.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   fpu/softfloat-parts.c.inc | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
>> index 45606f8402..914e588ec8 100644
>> --- a/fpu/softfloat-parts.c.inc
>> +++ b/fpu/softfloat-parts.c.inc
>> @@ -341,7 +341,17 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s,
>>           g_assert_not_reached();
>>       }
>>
>> -    exp = p->exp + fmt->exp_bias;
>> +    /* Because exp_bias is positive, we can only overflow past INT_MAX. */
>> +    if (sadd32_overflow(p->exp, fmt->exp_bias, &exp)) {
>> +        /*
>> +         * rebias_overflow wants to compute a modulo exponent, which
>> +         * conflicts with saturation.  That said, saturation can only
>> +         * happen with scalbn, which is not a PowerPC operation.
>> +         */
>> +        assert(!s->rebias_overflow);
>> +        exp = INT32_MAX;
> 
> As mentioned in the review comment on one of the Arm patches,
> I think that if we set exp to INT32_MAX here then the
> "round up" code below can increment it, overflowing to INT_MIN,
> and then we won't correctly spot that it is too large for the format.

Right-o.  I'm going to drop this one and constrain exp during scalbn et al to something 
smaller than INT32_{MIN,MAX}, so that we don't have to consider overflow everywhere.


r~
Re: [PATCH v4 11/12] fpu: Saturate the exponent in uncanon_normal
Posted by Philippe Mathieu-Daudé 2 months ago
On 17/5/26 02:25, Richard Henderson wrote:
> Notice the conflict between saturation and rebias_overflow,
> which should never happen.  Otherwise, saturate to INT32_MAX
> and allow the usual overflow to max, infinity, or dnan.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   fpu/softfloat-parts.c.inc | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/fpu/softfloat-parts.c.inc b/fpu/softfloat-parts.c.inc
> index 45606f8402..914e588ec8 100644
> --- a/fpu/softfloat-parts.c.inc
> +++ b/fpu/softfloat-parts.c.inc
> @@ -341,7 +341,17 @@ static void partsN(uncanon_normal)(FloatPartsN *p, float_status *s,
>           g_assert_not_reached();
>       }
>   
> -    exp = p->exp + fmt->exp_bias;
> +    /* Because exp_bias is positive, we can only overflow past INT_MAX. */
> +    if (sadd32_overflow(p->exp, fmt->exp_bias, &exp)) {
> +        /*
> +         * rebias_overflow wants to compute a modulo exponent, which
> +         * conflicts with saturation.  That said, saturation can only
> +         * happen with scalbn, which is not a PowerPC operation.
> +         */
> +        assert(!s->rebias_overflow);
> +        exp = INT32_MAX;
> +    }
> +
>       if (likely(exp > 0)) {
>           if (p->frac_lo & round_mask) {
>               flags |= float_flag_inexact;

To the best of my softfloat understanding:
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>