diff mbox series

[v5,28/28] fpu/softfloat: Define floatN_silence_nan in terms of parts_silence_nan

Message ID 20180514221219.7091-29-richard.henderson@linaro.org
State Superseded
Headers show
Series softfloat patch roundup | expand

Commit Message

Richard Henderson May 14, 2018, 10:12 p.m. UTC
Isolate the target-specific choice to 3 functions instead of 6.

The code in floatx80_default_nan tried to be over-general.  There are
only two targets that support this format: x86 and m68k.  Thus there
is no point in inventing a mechanism for snan_bit_is_one.

Move routines that no longer have ifdefs out of softfloat-specialize.h.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 fpu/softfloat-specialize.h | 81 ++------------------------------------
 fpu/softfloat.c            | 19 +++++++++
 2 files changed, 23 insertions(+), 77 deletions(-)

-- 
2.17.0

Comments

Alex Bennée May 15, 2018, 1:45 p.m. UTC | #1
Richard Henderson <richard.henderson@linaro.org> writes:

> Isolate the target-specific choice to 3 functions instead of 6.

>

> The code in floatx80_default_nan tried to be over-general.  There are

> only two targets that support this format: x86 and m68k.  Thus there

> is no point in inventing a mechanism for snan_bit_is_one.

>

> Move routines that no longer have ifdefs out of softfloat-specialize.h.

>

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

> ---

>  fpu/softfloat-specialize.h | 81 ++------------------------------------

>  fpu/softfloat.c            | 19 +++++++++

>  2 files changed, 23 insertions(+), 77 deletions(-)

>

> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h

> index ec4fb6ba8b..16c0bcb6fa 100644

> --- a/fpu/softfloat-specialize.h

> +++ b/fpu/softfloat-specialize.h

> @@ -278,24 +278,6 @@ int float16_is_signaling_nan(float16 a_, float_status *status)

>  #endif

>  }

>

> -/*----------------------------------------------------------------------------

> -| Returns a quiet NaN from a signalling NaN for the half-precision

> -| floating point value `a'.

> -*----------------------------------------------------------------------------*/

> -

> -float16 float16_silence_nan(float16 a, float_status *status)

> -{

> -#ifdef NO_SIGNALING_NANS

> -    g_assert_not_reached();

> -#else

> -    if (snan_bit_is_one(status)) {

> -        return float16_default_nan(status);

> -    } else {

> -        return a | (1 << 9);

> -    }

> -#endif

> -}

> -

>  /*----------------------------------------------------------------------------

>  | Returns 1 if the single-precision floating-point value `a' is a quiet

>  | NaN; otherwise returns 0.

> @@ -334,30 +316,6 @@ int float32_is_signaling_nan(float32 a_, float_status *status)

>  #endif

>  }

>

> -/*----------------------------------------------------------------------------

> -| Returns a quiet NaN from a signalling NaN for the single-precision

> -| floating point value `a'.

> -*----------------------------------------------------------------------------*/

> -

> -float32 float32_silence_nan(float32 a, float_status *status)

> -{

> -#ifdef NO_SIGNALING_NANS

> -    g_assert_not_reached();

> -#else

> -    if (snan_bit_is_one(status)) {

> -# ifdef TARGET_HPPA

> -        a &= ~0x00400000;

> -        a |=  0x00200000;

> -        return a;

> -# else

> -        return float32_default_nan(status);

> -# endif

> -    } else {

> -        return a | (1 << 22);

> -    }

> -#endif

> -}

> -

>  /*----------------------------------------------------------------------------

>  | Returns the result of converting the single-precision floating-point NaN

>  | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid

> @@ -706,31 +664,6 @@ int float64_is_signaling_nan(float64 a_, float_status *status)

>  #endif

>  }

>

> -/*----------------------------------------------------------------------------

> -| Returns a quiet NaN from a signalling NaN for the double-precision

> -| floating point value `a'.

> -*----------------------------------------------------------------------------*/

> -

> -float64 float64_silence_nan(float64 a, float_status *status)

> -{

> -#ifdef NO_SIGNALING_NANS

> -    g_assert_not_reached();

> -#else

> -    if (snan_bit_is_one(status)) {

> -# ifdef TARGET_HPPA

> -        a &= ~0x0008000000000000ULL;

> -        a |=  0x0004000000000000ULL;

> -        return a;

> -# else

> -        return float64_default_nan(status);

> -# endif

> -    } else {

> -        return a | LIT64(0x0008000000000000);

> -    }

> -#endif

> -}

> -

> -

>  /*----------------------------------------------------------------------------

>  | Returns the result of converting the double-precision floating-point NaN

>  | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid

> @@ -886,16 +819,10 @@ int floatx80_is_signaling_nan(floatx80 a, float_status *status)

>

>  floatx80 floatx80_silence_nan(floatx80 a, float_status *status)

>  {

> -#ifdef NO_SIGNALING_NANS

> -    g_assert_not_reached();

> -#else

> -    if (snan_bit_is_one(status)) {

> -        return floatx80_default_nan(status);

> -    } else {

> -        a.low |= LIT64(0xC000000000000000);

> -        return a;

> -    }

> -#endif

> +    /* None of the targets that have snan_bit_is_one use floatx80.  */

> +    assert(!snan_bit_is_one(status));

> +    a.low |= LIT64(0xC000000000000000);

> +    return a;

>  }

>

>  /*----------------------------------------------------------------------------

> diff --git a/fpu/softfloat.c b/fpu/softfloat.c

> index b5842f7b1c..40b039ee5b 100644

> --- a/fpu/softfloat.c

> +++ b/fpu/softfloat.c

> @@ -2128,6 +2128,25 @@ float128 float128_default_nan(float_status *status)

>      return r;

>  }

>

> +/*----------------------------------------------------------------------------

> +| Returns a quiet NaN from a signalling NaN for the floating point value `a'.

> +*----------------------------------------------------------------------------*/

> +

> +float16 float16_silence_nan(float16 a, float_status *status)

> +{

> +    return float16_pack_raw(parts_silence_nan(float16_unpack_raw(a), status));

> +}

> +

> +float32 float32_silence_nan(float32 a, float_status *status)

> +{

> +    return float32_pack_raw(parts_silence_nan(float32_unpack_raw(a), status));

> +}

> +

> +float64 float64_silence_nan(float64 a, float_status *status)

> +{

> +    return float64_pack_raw(parts_silence_nan(float64_unpack_raw(a), status));

> +}

> +


Not that I'm objecting to the rationalisation but did you look at the
code generated now we unpack NaNs? I guess NaN behaviour isn't the
critical path for performance anyway....

Anyway:

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



>  /*----------------------------------------------------------------------------

>  | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6

>  | and 7, and returns the properly rounded 32-bit integer corresponding to the



--
Alex Bennée
Richard Henderson May 15, 2018, 3:41 p.m. UTC | #2
On 05/15/2018 06:45 AM, Alex Bennée wrote:
>> +float64 float64_silence_nan(float64 a, float_status *status)

>> +{

>> +    return float64_pack_raw(parts_silence_nan(float64_unpack_raw(a), status));

>> +}

>> +

> 

> Not that I'm objecting to the rationalisation but did you look at the

> code generated now we unpack NaNs? I guess NaN behaviour isn't the

> critical path for performance anyway....


Yes, I looked.  It's about 5 instructions instead of 1.
But as you say, it's nowhere near critical path.

Ug.  I've also just realized that the shift isn't correct though...


r~
Richard Henderson May 15, 2018, 4:14 p.m. UTC | #3
On 05/15/2018 08:41 AM, Richard Henderson wrote:
> On 05/15/2018 06:45 AM, Alex Bennée wrote:

>>> +float64 float64_silence_nan(float64 a, float_status *status)

>>> +{

>>> +    return float64_pack_raw(parts_silence_nan(float64_unpack_raw(a), status));

>>> +}

>>> +

>>

>> Not that I'm objecting to the rationalisation but did you look at the

>> code generated now we unpack NaNs? I guess NaN behaviour isn't the

>> critical path for performance anyway....

> 

> Yes, I looked.  It's about 5 instructions instead of 1.

> But as you say, it's nowhere near critical path.

> 

> Ug.  I've also just realized that the shift isn't correct though...


Having fixed that and re-checked... the compiler is weird.

The float32 version optimizes to 1 insn, as we would hope.  The float16 version
optimizes to 5 insns, extracting and re-inserting the sign bit.  The float64
version optimizes to 10 insns, extracting and re-inserting the exponent as well.

Very odd.


r~
diff mbox series

Patch

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index ec4fb6ba8b..16c0bcb6fa 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -278,24 +278,6 @@  int float16_is_signaling_nan(float16 a_, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN from a signalling NaN for the half-precision
-| floating point value `a'.
-*----------------------------------------------------------------------------*/
-
-float16 float16_silence_nan(float16 a, float_status *status)
-{
-#ifdef NO_SIGNALING_NANS
-    g_assert_not_reached();
-#else
-    if (snan_bit_is_one(status)) {
-        return float16_default_nan(status);
-    } else {
-        return a | (1 << 9);
-    }
-#endif
-}
-
 /*----------------------------------------------------------------------------
 | Returns 1 if the single-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
@@ -334,30 +316,6 @@  int float32_is_signaling_nan(float32 a_, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN from a signalling NaN for the single-precision
-| floating point value `a'.
-*----------------------------------------------------------------------------*/
-
-float32 float32_silence_nan(float32 a, float_status *status)
-{
-#ifdef NO_SIGNALING_NANS
-    g_assert_not_reached();
-#else
-    if (snan_bit_is_one(status)) {
-# ifdef TARGET_HPPA
-        a &= ~0x00400000;
-        a |=  0x00200000;
-        return a;
-# else
-        return float32_default_nan(status);
-# endif
-    } else {
-        return a | (1 << 22);
-    }
-#endif
-}
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the single-precision floating-point NaN
 | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
@@ -706,31 +664,6 @@  int float64_is_signaling_nan(float64 a_, float_status *status)
 #endif
 }
 
-/*----------------------------------------------------------------------------
-| Returns a quiet NaN from a signalling NaN for the double-precision
-| floating point value `a'.
-*----------------------------------------------------------------------------*/
-
-float64 float64_silence_nan(float64 a, float_status *status)
-{
-#ifdef NO_SIGNALING_NANS
-    g_assert_not_reached();
-#else
-    if (snan_bit_is_one(status)) {
-# ifdef TARGET_HPPA
-        a &= ~0x0008000000000000ULL;
-        a |=  0x0004000000000000ULL;
-        return a;
-# else
-        return float64_default_nan(status);
-# endif
-    } else {
-        return a | LIT64(0x0008000000000000);
-    }
-#endif
-}
-
-
 /*----------------------------------------------------------------------------
 | Returns the result of converting the double-precision floating-point NaN
 | `a' to the canonical NaN format.  If `a' is a signaling NaN, the invalid
@@ -886,16 +819,10 @@  int floatx80_is_signaling_nan(floatx80 a, float_status *status)
 
 floatx80 floatx80_silence_nan(floatx80 a, float_status *status)
 {
-#ifdef NO_SIGNALING_NANS
-    g_assert_not_reached();
-#else
-    if (snan_bit_is_one(status)) {
-        return floatx80_default_nan(status);
-    } else {
-        a.low |= LIT64(0xC000000000000000);
-        return a;
-    }
-#endif
+    /* None of the targets that have snan_bit_is_one use floatx80.  */
+    assert(!snan_bit_is_one(status));
+    a.low |= LIT64(0xC000000000000000);
+    return a;
 }
 
 /*----------------------------------------------------------------------------
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index b5842f7b1c..40b039ee5b 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -2128,6 +2128,25 @@  float128 float128_default_nan(float_status *status)
     return r;
 }
 
+/*----------------------------------------------------------------------------
+| Returns a quiet NaN from a signalling NaN for the floating point value `a'.
+*----------------------------------------------------------------------------*/
+
+float16 float16_silence_nan(float16 a, float_status *status)
+{
+    return float16_pack_raw(parts_silence_nan(float16_unpack_raw(a), status));
+}
+
+float32 float32_silence_nan(float32 a, float_status *status)
+{
+    return float32_pack_raw(parts_silence_nan(float32_unpack_raw(a), status));
+}
+
+float64 float64_silence_nan(float64 a, float_status *status)
+{
+    return float64_pack_raw(parts_silence_nan(float64_unpack_raw(a), status));
+}
+
 /*----------------------------------------------------------------------------
 | Takes a 64-bit fixed-point value `absZ' with binary point between bits 6
 | and 7, and returns the properly rounded 32-bit integer corresponding to the