Message ID | 1569851517-5682-1-git-send-email-pc@us.ibm.com |
---|---|
State | New |
Headers | show |
Series | [v2] Enable inlining issignalingf within glibc | expand |
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
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)
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(-)