diff mbox series

[07/11] softfloat: Use parts_pick_nan in propagateFloatx80NaN

Message ID 20241203203949.483774-8-richard.henderson@linaro.org
State New
Headers show
Series fpu: pickNaN follow ups | expand

Commit Message

Richard Henderson Dec. 3, 2024, 8:39 p.m. UTC 
Unpacking and repacking the parts may be slightly more work
than we did before, but we get to reuse more code.  For a
code path handling exceptional values, this is an improvement.

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

Comments

Peter Maydell Dec. 10, 2024, 5:10 p.m. UTC | #1 
On Tue, 3 Dec 2024 at 20:39, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Unpacking and repacking the parts may be slightly more work
> than we did before, but we get to reuse more code.  For a
> code path handling exceptional values, this is an improvement.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  fpu/softfloat.c | 43 +++++--------------------------------------
>  1 file changed, 5 insertions(+), 38 deletions(-)
>
> diff --git a/fpu/softfloat.c b/fpu/softfloat.c
> index 6ba1cfd32a..8de8d5f342 100644
> --- a/fpu/softfloat.c
> +++ b/fpu/softfloat.c
> @@ -4928,48 +4928,15 @@ void normalizeFloatx80Subnormal(uint64_t aSig, int32_t *zExpPtr,
>
>  floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)

This is a curious function, because although it looks like it
ought to be part of softfloat itself in fact it is used in
exactly one function in target/m68k: floatx80_scale().

>  {
> -    bool aIsLargerSignificand;
> -    FloatClass a_cls, b_cls;
> +    FloatParts128 pa, pb, *pr;
>
> -    /* This is not complete, but is good enough for pickNaN.  */
> -    a_cls = (!floatx80_is_any_nan(a)
> -             ? float_class_normal
> -             : floatx80_is_signaling_nan(a, status)
> -             ? float_class_snan
> -             : float_class_qnan);
> -    b_cls = (!floatx80_is_any_nan(b)
> -             ? float_class_normal
> -             : floatx80_is_signaling_nan(b, status)
> -             ? float_class_snan
> -             : float_class_qnan);
> -
> -    if (is_snan(a_cls) || is_snan(b_cls)) {
> -        float_raise(float_flag_invalid, status);
> -    }
> -
> -    if (status->default_nan_mode) {
> +    if (!floatx80_unpack_canonical(&pa, a, status) ||
> +        !floatx80_unpack_canonical(&pb, b, status)) {
>          return floatx80_default_nan(status);
>      }
>
> -    if (a.low < b.low) {
> -        aIsLargerSignificand = 0;
> -    } else if (b.low < a.low) {
> -        aIsLargerSignificand = 1;
> -    } else {
> -        aIsLargerSignificand = (a.high < b.high) ? 1 : 0;
> -    }
> -
> -    if (pickNaN(a_cls, b_cls, aIsLargerSignificand, status)) {
> -        if (is_snan(b_cls)) {
> -            return floatx80_silence_nan(b, status);
> -        }
> -        return b;
> -    } else {
> -        if (is_snan(a_cls)) {
> -            return floatx80_silence_nan(a, status);
> -        }
> -        return a;
> -    }
> +    pr = parts_pick_nan(&pa, &pb, status);
> +    return floatx80_round_pack_canonical(pr, status);
>  }

If we were using this on anything except m68k this would
be a behaviour change for invalid-encoding floatx80,
but m68k currently makes floatx80_invalid_encoding()
always return false. So I think this should be OK:

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

-- PMM
diff mbox series

Patch

diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 6ba1cfd32a..8de8d5f342 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -4928,48 +4928,15 @@  void normalizeFloatx80Subnormal(uint64_t aSig, int32_t *zExpPtr,
 
 floatx80 propagateFloatx80NaN(floatx80 a, floatx80 b, float_status *status)
 {
-    bool aIsLargerSignificand;
-    FloatClass a_cls, b_cls;
+    FloatParts128 pa, pb, *pr;
 
-    /* This is not complete, but is good enough for pickNaN.  */
-    a_cls = (!floatx80_is_any_nan(a)
-             ? float_class_normal
-             : floatx80_is_signaling_nan(a, status)
-             ? float_class_snan
-             : float_class_qnan);
-    b_cls = (!floatx80_is_any_nan(b)
-             ? float_class_normal
-             : floatx80_is_signaling_nan(b, status)
-             ? float_class_snan
-             : float_class_qnan);
-
-    if (is_snan(a_cls) || is_snan(b_cls)) {
-        float_raise(float_flag_invalid, status);
-    }
-
-    if (status->default_nan_mode) {
+    if (!floatx80_unpack_canonical(&pa, a, status) ||
+        !floatx80_unpack_canonical(&pb, b, status)) {
         return floatx80_default_nan(status);
     }
 
-    if (a.low < b.low) {
-        aIsLargerSignificand = 0;
-    } else if (b.low < a.low) {
-        aIsLargerSignificand = 1;
-    } else {
-        aIsLargerSignificand = (a.high < b.high) ? 1 : 0;
-    }
-
-    if (pickNaN(a_cls, b_cls, aIsLargerSignificand, status)) {
-        if (is_snan(b_cls)) {
-            return floatx80_silence_nan(b, status);
-        }
-        return b;
-    } else {
-        if (is_snan(a_cls)) {
-            return floatx80_silence_nan(a, status);
-        }
-        return a;
-    }
+    pr = parts_pick_nan(&pa, &pb, status);
+    return floatx80_round_pack_canonical(pr, status);
 }
 
 /*----------------------------------------------------------------------------