[v2] Enable inlining issignalingf within glibc
Commit Message
From: "Paul A. Clarke" <pc@us.ibm.com>
issignalingf is a very small function used in some areas where
better performance (and smaller code) might be helpful.
Create inline implementation for issignalingf.
2019-09-30 Paul A. Clarke <pc@us.ibm.com>
* sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
Moved...
* include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
(__issignalingf): New, copied from
sysdeps/iee754/flt-32/s_issignalingf.c.
---
v2:
- New approach as suggested by Joseph Myers: no implementations in
math_private.h, instead in include/math.h.
- Moved GET_FLOAT_WORD from math_private.h to include/math.h in support.
- Brought along matching SET_FLOAT_WORD for consistency.
include/math.h | 53 ++++++++++++++++++++++++++++++++++++++++++
sysdeps/generic/math_private.h | 29 -----------------------
2 files changed, 53 insertions(+), 29 deletions(-)
Comments
Sorry, this should've been noted as "v3". ugh
On 9/30/19 8:51 AM, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>
>
> issignalingf is a very small function used in some areas where
> better performance (and smaller code) might be helpful.
>
> Create inline implementation for issignalingf.
>
> 2019-09-30 Paul A. Clarke <pc@us.ibm.com>
>
> * sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
> Moved...
> * include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
> (__issignalingf): New, copied from
> sysdeps/iee754/flt-32/s_issignalingf.c.
> ---
> v2:
> - New approach as suggested by Joseph Myers: no implementations in
> math_private.h, instead in include/math.h.
> - Moved GET_FLOAT_WORD from math_private.h to include/math.h in support.
> - Brought along matching SET_FLOAT_WORD for consistency.
>
> include/math.h | 53 ++++++++++++++++++++++++++++++++++++++++++
> sysdeps/generic/math_private.h | 29 -----------------------
> 2 files changed, 53 insertions(+), 29 deletions(-)
>
> diff --git a/include/math.h b/include/math.h
> index 79ebbae..a274f2b 100644
> --- a/include/math.h
> +++ b/include/math.h
> @@ -54,6 +54,59 @@ libm_hidden_proto (__expf128)
> libm_hidden_proto (__expm1f128)
> # endif
>
> +#include <stdint.h>
> +#include <nan-high-order-bit.h>
> +
> +/* A union which permits us to convert between a float and a 32 bit
> + int. */
> +
> +typedef union
> +{
> + float value;
> + uint32_t word;
> +} ieee_float_shape_type;
> +
> +/* Get a 32 bit int from a float. */
> +#ifndef GET_FLOAT_WORD
> +# define GET_FLOAT_WORD(i,d) \
> +do { \
> + ieee_float_shape_type gf_u; \
> + gf_u.value = (d); \
> + (i) = gf_u.word; \
> +} while (0)
> +#endif
> +
> +/* Set a float from a 32 bit int. */
> +#ifndef SET_FLOAT_WORD
> +# define SET_FLOAT_WORD(d,i) \
> +do { \
> + ieee_float_shape_type sf_u; \
> + sf_u.word = (i); \
> + (d) = sf_u.value; \
> +} while (0)
> +#endif
> +
> +extern inline int
> +__issignalingf (float x)
> +{
> + uint32_t xi;
> + GET_FLOAT_WORD (xi, x);
> +#if HIGH_ORDER_BIT_IS_SET_FOR_SNAN
> + /* We only have to care about the high-order bit of x's significand, because
> + having it set (sNaN) already makes the significand different from that
> + used to designate infinity. */
> + return (xi & 0x7fc00000) == 0x7fc00000;
> +#else
> + /* To keep the following comparison simple, toggle the quiet/signaling bit,
> + so that it is set for sNaNs. This is inverse to IEEE 754-2008 (as well as
> + common practice for IEEE 754-1985). */
> + xi ^= 0x00400000;
> + /* We have to compare for greater (instead of greater or equal), because x's
> + significand being all-zero designates infinity not NaN. */
> + return (xi & 0x7fffffff) > 0x7fc00000;
> +#endif
> +}
> +
> # if __HAVE_DISTINCT_FLOAT128
>
> /* __builtin_isinf_sign is broken in GCC < 7 for float128. */
> diff --git a/sysdeps/generic/math_private.h b/sysdeps/generic/math_private.h
> index d91b929..9296324 100644
> --- a/sysdeps/generic/math_private.h
> +++ b/sysdeps/generic/math_private.h
> @@ -153,35 +153,6 @@ do { \
> } while (0)
> #endif
>
> -/* A union which permits us to convert between a float and a 32 bit
> - int. */
> -
> -typedef union
> -{
> - float value;
> - uint32_t word;
> -} ieee_float_shape_type;
> -
> -/* Get a 32 bit int from a float. */
> -#ifndef GET_FLOAT_WORD
> -# define GET_FLOAT_WORD(i,d) \
> -do { \
> - ieee_float_shape_type gf_u; \
> - gf_u.value = (d); \
> - (i) = gf_u.word; \
> -} while (0)
> -#endif
> -
> -/* Set a float from a 32 bit int. */
> -#ifndef SET_FLOAT_WORD
> -# define SET_FLOAT_WORD(d,i) \
> -do { \
> - ieee_float_shape_type sf_u; \
> - sf_u.word = (i); \
> - (d) = sf_u.value; \
> -} while (0)
> -#endif
> -
> /* We need to guarantee an expansion of name when building
> ldbl-128 files as another type (e.g _Float128). */
> #define mathx_hidden_def(name) hidden_def(name)
>
PC
ping.
On 9/30/19 10:19 AM, Paul Clarke wrote:
> Sorry, this should've been noted as "v3". ugh
>
> On 9/30/19 8:51 AM, Paul A. Clarke wrote:
>> From: "Paul A. Clarke" <pc@us.ibm.com>
>>
>> issignalingf is a very small function used in some areas where
>> better performance (and smaller code) might be helpful.
>>
>> Create inline implementation for issignalingf.
>>
>> 2019-09-30 Paul A. Clarke <pc@us.ibm.com>
>>
>> * sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
>> Moved...
>> * include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
>> (__issignalingf): New, copied from
>> sysdeps/iee754/flt-32/s_issignalingf.c.
>> ---
>> v2:
>> - New approach as suggested by Joseph Myers: no implementations in
>> math_private.h, instead in include/math.h.
>> - Moved GET_FLOAT_WORD from math_private.h to include/math.h in support.
>> - Brought along matching SET_FLOAT_WORD for consistency.
>>
>> include/math.h | 53 ++++++++++++++++++++++++++++++++++++++++++
>> sysdeps/generic/math_private.h | 29 -----------------------
>> 2 files changed, 53 insertions(+), 29 deletions(-)
>>
>> diff --git a/include/math.h b/include/math.h
>> index 79ebbae..a274f2b 100644
>> --- a/include/math.h
>> +++ b/include/math.h
>> @@ -54,6 +54,59 @@ libm_hidden_proto (__expf128)
>> libm_hidden_proto (__expm1f128)
>> # endif
>>
>> +#include <stdint.h>
>> +#include <nan-high-order-bit.h>
>> +
>> +/* A union which permits us to convert between a float and a 32 bit
>> + int. */
>> +
>> +typedef union
>> +{
>> + float value;
>> + uint32_t word;
>> +} ieee_float_shape_type;
>> +
>> +/* Get a 32 bit int from a float. */
>> +#ifndef GET_FLOAT_WORD
>> +# define GET_FLOAT_WORD(i,d) \
>> +do { \
>> + ieee_float_shape_type gf_u; \
>> + gf_u.value = (d); \
>> + (i) = gf_u.word; \
>> +} while (0)
>> +#endif
>> +
>> +/* Set a float from a 32 bit int. */
>> +#ifndef SET_FLOAT_WORD
>> +# define SET_FLOAT_WORD(d,i) \
>> +do { \
>> + ieee_float_shape_type sf_u; \
>> + sf_u.word = (i); \
>> + (d) = sf_u.value; \
>> +} while (0)
>> +#endif
>> +
>> +extern inline int
>> +__issignalingf (float x)
>> +{
>> + uint32_t xi;
>> + GET_FLOAT_WORD (xi, x);
>> +#if HIGH_ORDER_BIT_IS_SET_FOR_SNAN
>> + /* We only have to care about the high-order bit of x's significand, because
>> + having it set (sNaN) already makes the significand different from that
>> + used to designate infinity. */
>> + return (xi & 0x7fc00000) == 0x7fc00000;
>> +#else
>> + /* To keep the following comparison simple, toggle the quiet/signaling bit,
>> + so that it is set for sNaNs. This is inverse to IEEE 754-2008 (as well as
>> + common practice for IEEE 754-1985). */
>> + xi ^= 0x00400000;
>> + /* We have to compare for greater (instead of greater or equal), because x's
>> + significand being all-zero designates infinity not NaN. */
>> + return (xi & 0x7fffffff) > 0x7fc00000;
>> +#endif
>> +}
>> +
>> # if __HAVE_DISTINCT_FLOAT128
>>
>> /* __builtin_isinf_sign is broken in GCC < 7 for float128. */
>> diff --git a/sysdeps/generic/math_private.h b/sysdeps/generic/math_private.h
>> index d91b929..9296324 100644
>> --- a/sysdeps/generic/math_private.h
>> +++ b/sysdeps/generic/math_private.h
>> @@ -153,35 +153,6 @@ do { \
>> } while (0)
>> #endif
>>
>> -/* A union which permits us to convert between a float and a 32 bit
>> - int. */
>> -
>> -typedef union
>> -{
>> - float value;
>> - uint32_t word;
>> -} ieee_float_shape_type;
>> -
>> -/* Get a 32 bit int from a float. */
>> -#ifndef GET_FLOAT_WORD
>> -# define GET_FLOAT_WORD(i,d) \
>> -do { \
>> - ieee_float_shape_type gf_u; \
>> - gf_u.value = (d); \
>> - (i) = gf_u.word; \
>> -} while (0)
>> -#endif
>> -
>> -/* Set a float from a 32 bit int. */
>> -#ifndef SET_FLOAT_WORD
>> -# define SET_FLOAT_WORD(d,i) \
>> -do { \
>> - ieee_float_shape_type sf_u; \
>> - sf_u.word = (i); \
>> - (d) = sf_u.value; \
>> -} while (0)
>> -#endif
>> -
>> /* We need to guarantee an expansion of name when building
>> ldbl-128 files as another type (e.g _Float128). */
>> #define mathx_hidden_def(name) hidden_def(name)
>>
> PC
>
ping #2.
On 10/16/19 11:50 AM, Paul Clarke wrote:
> ping.
>
> On 9/30/19 10:19 AM, Paul Clarke wrote:
>> Sorry, this should've been noted as "v3". ugh
>>
>> On 9/30/19 8:51 AM, Paul A. Clarke wrote:
>>> From: "Paul A. Clarke" <pc@us.ibm.com>
>>>
>>> issignalingf is a very small function used in some areas where
>>> better performance (and smaller code) might be helpful.
>>>
>>> Create inline implementation for issignalingf.
>>>
>>> 2019-09-30 Paul A. Clarke <pc@us.ibm.com>
>>>
>>> * sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
>>> Moved...
>>> * include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
>>> (__issignalingf): New, copied from
>>> sysdeps/iee754/flt-32/s_issignalingf.c.
>>> ---
>>> v2:
>>> - New approach as suggested by Joseph Myers: no implementations in
>>> math_private.h, instead in include/math.h.
>>> - Moved GET_FLOAT_WORD from math_private.h to include/math.h in support.
>>> - Brought along matching SET_FLOAT_WORD for consistency.
>>>
>>> include/math.h | 53 ++++++++++++++++++++++++++++++++++++++++++
>>> sysdeps/generic/math_private.h | 29 -----------------------
>>> 2 files changed, 53 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/include/math.h b/include/math.h
>>> index 79ebbae..a274f2b 100644
>>> --- a/include/math.h
>>> +++ b/include/math.h
>>> @@ -54,6 +54,59 @@ libm_hidden_proto (__expf128)
>>> libm_hidden_proto (__expm1f128)
>>> # endif
>>>
>>> +#include <stdint.h>
>>> +#include <nan-high-order-bit.h>
>>> +
>>> +/* A union which permits us to convert between a float and a 32 bit
>>> + int. */
>>> +
>>> +typedef union
>>> +{
>>> + float value;
>>> + uint32_t word;
>>> +} ieee_float_shape_type;
>>> +
>>> +/* Get a 32 bit int from a float. */
>>> +#ifndef GET_FLOAT_WORD
>>> +# define GET_FLOAT_WORD(i,d) \
>>> +do { \
>>> + ieee_float_shape_type gf_u; \
>>> + gf_u.value = (d); \
>>> + (i) = gf_u.word; \
>>> +} while (0)
>>> +#endif
>>> +
>>> +/* Set a float from a 32 bit int. */
>>> +#ifndef SET_FLOAT_WORD
>>> +# define SET_FLOAT_WORD(d,i) \
>>> +do { \
>>> + ieee_float_shape_type sf_u; \
>>> + sf_u.word = (i); \
>>> + (d) = sf_u.value; \
>>> +} while (0)
>>> +#endif
>>> +
>>> +extern inline int
>>> +__issignalingf (float x)
>>> +{
>>> + uint32_t xi;
>>> + GET_FLOAT_WORD (xi, x);
>>> +#if HIGH_ORDER_BIT_IS_SET_FOR_SNAN
>>> + /* We only have to care about the high-order bit of x's significand, because
>>> + having it set (sNaN) already makes the significand different from that
>>> + used to designate infinity. */
>>> + return (xi & 0x7fc00000) == 0x7fc00000;
>>> +#else
>>> + /* To keep the following comparison simple, toggle the quiet/signaling bit,
>>> + so that it is set for sNaNs. This is inverse to IEEE 754-2008 (as well as
>>> + common practice for IEEE 754-1985). */
>>> + xi ^= 0x00400000;
>>> + /* We have to compare for greater (instead of greater or equal), because x's
>>> + significand being all-zero designates infinity not NaN. */
>>> + return (xi & 0x7fffffff) > 0x7fc00000;
>>> +#endif
>>> +}
>>> +
>>> # if __HAVE_DISTINCT_FLOAT128
>>>
>>> /* __builtin_isinf_sign is broken in GCC < 7 for float128. */
>>> diff --git a/sysdeps/generic/math_private.h b/sysdeps/generic/math_private.h
>>> index d91b929..9296324 100644
>>> --- a/sysdeps/generic/math_private.h
>>> +++ b/sysdeps/generic/math_private.h
>>> @@ -153,35 +153,6 @@ do { \
>>> } while (0)
>>> #endif
>>>
>>> -/* A union which permits us to convert between a float and a 32 bit
>>> - int. */
>>> -
>>> -typedef union
>>> -{
>>> - float value;
>>> - uint32_t word;
>>> -} ieee_float_shape_type;
>>> -
>>> -/* Get a 32 bit int from a float. */
>>> -#ifndef GET_FLOAT_WORD
>>> -# define GET_FLOAT_WORD(i,d) \
>>> -do { \
>>> - ieee_float_shape_type gf_u; \
>>> - gf_u.value = (d); \
>>> - (i) = gf_u.word; \
>>> -} while (0)
>>> -#endif
>>> -
>>> -/* Set a float from a 32 bit int. */
>>> -#ifndef SET_FLOAT_WORD
>>> -# define SET_FLOAT_WORD(d,i) \
>>> -do { \
>>> - ieee_float_shape_type sf_u; \
>>> - sf_u.word = (i); \
>>> - (d) = sf_u.value; \
>>> -} while (0)
>>> -#endif
>>> -
>>> /* We need to guarantee an expansion of name when building
>>> ldbl-128 files as another type (e.g _Float128). */
>>> #define mathx_hidden_def(name) hidden_def(name)
>>>
>> PC
>>
On Mon, 30 Sep 2019, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>
>
> issignalingf is a very small function used in some areas where
> better performance (and smaller code) might be helpful.
>
> Create inline implementation for issignalingf.
>
> 2019-09-30 Paul A. Clarke <pc@us.ibm.com>
>
> * sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
> Moved...
> * include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
> (__issignalingf): New, copied from
> sysdeps/iee754/flt-32/s_issignalingf.c.
This is OK.
There's the question of what to do about issignalingf_inline in
sysdeps/ieee754/flt-32/math_config.h and its uses in
sysdeps/ieee754/flt-32/e_powf.c. Arguably they ought to move to just
calling issignaling. If however the slightly different code in
issignalingf_inline results in better code in the callers (which would
need to be checked, I don't know whether masking or shifting would be
better in general), that would indicate (a) changing the code in the
generic inline and (b) filing a GCC bug report to convert one into the
other as an optimization.
On 10/29/19 5:18 PM, Joseph Myers wrote:
> On Mon, 30 Sep 2019, Paul A. Clarke wrote:
>> issignalingf is a very small function used in some areas where
>> better performance (and smaller code) might be helpful.
>>
>> Create inline implementation for issignalingf.
>>
>> 2019-09-30 Paul A. Clarke <pc@us.ibm.com>
>>
>> * sysdeps/generic/math_private.h (GET_FLOAT_WORD, SET_FLOAT_WORD):
>> Moved...
>> * include/math.h (GET_FLOAT_WORD, SET_FLOAT_WORD): to here.
>> (__issignalingf): New, copied from
>> sysdeps/iee754/flt-32/s_issignalingf.c.
>
> This is OK.
>
> There's the question of what to do about issignalingf_inline in
> sysdeps/ieee754/flt-32/math_config.h and its uses in
> sysdeps/ieee754/flt-32/e_powf.c. Arguably they ought to move to just
> calling issignaling. If however the slightly different code in
> issignalingf_inline results in better code in the callers (which would
> need to be checked, I don't know whether masking or shifting would be
> better in general), that would indicate (a) changing the code in the
> generic inline and (b) filing a GCC bug report to convert one into the
> other as an optimization.
>
The generated code was identical on ppc64le. There were a few minor differences on x86_64 (GCC7 and GCC8; looking at just the mnemonics; '-' is current issignalingf_inline, '+' is new __issignalingf):
--
@@ -210,7 +210,7 @@
xor
movss
-add
+and
cmp
jbe
mov
@@ -244,10 +244,9 @@
movaps
retq
xor
-lea
+and
cmp
ja
movss
---
...but I am far from an expert as to which is better on x86_64.
PC
@@ -54,6 +54,59 @@ libm_hidden_proto (__expf128)
libm_hidden_proto (__expm1f128)
# endif
+#include <stdint.h>
+#include <nan-high-order-bit.h>
+
+/* A union which permits us to convert between a float and a 32 bit
+ int. */
+
+typedef union
+{
+ float value;
+ uint32_t word;
+} ieee_float_shape_type;
+
+/* Get a 32 bit int from a float. */
+#ifndef GET_FLOAT_WORD
+# define GET_FLOAT_WORD(i,d) \
+do { \
+ ieee_float_shape_type gf_u; \
+ gf_u.value = (d); \
+ (i) = gf_u.word; \
+} while (0)
+#endif
+
+/* Set a float from a 32 bit int. */
+#ifndef SET_FLOAT_WORD
+# define SET_FLOAT_WORD(d,i) \
+do { \
+ ieee_float_shape_type sf_u; \
+ sf_u.word = (i); \
+ (d) = sf_u.value; \
+} while (0)
+#endif
+
+extern inline int
+__issignalingf (float x)
+{
+ uint32_t xi;
+ GET_FLOAT_WORD (xi, x);
+#if HIGH_ORDER_BIT_IS_SET_FOR_SNAN
+ /* We only have to care about the high-order bit of x's significand, because
+ having it set (sNaN) already makes the significand different from that
+ used to designate infinity. */
+ return (xi & 0x7fc00000) == 0x7fc00000;
+#else
+ /* To keep the following comparison simple, toggle the quiet/signaling bit,
+ so that it is set for sNaNs. This is inverse to IEEE 754-2008 (as well as
+ common practice for IEEE 754-1985). */
+ xi ^= 0x00400000;
+ /* We have to compare for greater (instead of greater or equal), because x's
+ significand being all-zero designates infinity not NaN. */
+ return (xi & 0x7fffffff) > 0x7fc00000;
+#endif
+}
+
# if __HAVE_DISTINCT_FLOAT128
/* __builtin_isinf_sign is broken in GCC < 7 for float128. */
@@ -153,35 +153,6 @@ do { \
} while (0)
#endif
-/* A union which permits us to convert between a float and a 32 bit
- int. */
-
-typedef union
-{
- float value;
- uint32_t word;
-} ieee_float_shape_type;
-
-/* Get a 32 bit int from a float. */
-#ifndef GET_FLOAT_WORD
-# define GET_FLOAT_WORD(i,d) \
-do { \
- ieee_float_shape_type gf_u; \
- gf_u.value = (d); \
- (i) = gf_u.word; \
-} while (0)
-#endif
-
-/* Set a float from a 32 bit int. */
-#ifndef SET_FLOAT_WORD
-# define SET_FLOAT_WORD(d,i) \
-do { \
- ieee_float_shape_type sf_u; \
- sf_u.word = (i); \
- (d) = sf_u.value; \
-} while (0)
-#endif
-
/* We need to guarantee an expansion of name when building
ldbl-128 files as another type (e.g _Float128). */
#define mathx_hidden_def(name) hidden_def(name)