Remove duplicate inline implementation of issignalingf

Message ID 1574359570-6508-1-git-send-email-pc@us.ibm.com
State Superseded
Headers show
Series
  • Remove duplicate inline implementation of issignalingf
Related show

Commit Message

Paul A. Clarke Nov. 21, 2019, 6:06 p.m.
From: "Paul A. Clarke" <pc@us.ibm.com>


Very recent commit 854e91bf6b4221f424ffa13b9ef50f35623b7b74 enabled
inline of issignalingf() in general (__issignalingf in include/math.h).
There is another implementation for an inline use of issignalingf
(issignalingf_inline in sysdeps/ieee754/flt-32/math_config.h)
which could instead make use of the new enablement.

Replace the use of issignalingf_inline with __issignaling.

The implementations are slightly different, and compile to slightly
different code, but I measured no significant performance difference.

The second implementation was brought to my attention by:
Suggested-by: Joseph Myers <joseph@codesourcery.com>

Signed-off-by: Paul A. Clarke <pc@us.ibm.com>

---
 sysdeps/ieee754/flt-32/e_powf.c      | 4 ++--
 sysdeps/ieee754/flt-32/math_config.h | 9 ---------
 2 files changed, 2 insertions(+), 11 deletions(-)

-- 
1.8.3.1

Comments

Adhemerval Zanella Nov. 21, 2019, 7:47 p.m. | #1
On 21/11/2019 15:06, Paul A. Clarke wrote:
> From: "Paul A. Clarke" <pc@us.ibm.com>

> 

> Very recent commit 854e91bf6b4221f424ffa13b9ef50f35623b7b74 enabled

> inline of issignalingf() in general (__issignalingf in include/math.h).

> There is another implementation for an inline use of issignalingf

> (issignalingf_inline in sysdeps/ieee754/flt-32/math_config.h)

> which could instead make use of the new enablement.

> 

> Replace the use of issignalingf_inline with __issignaling.

> 

> The implementations are slightly different, and compile to slightly

> different code, but I measured no significant performance difference.

> 

> The second implementation was brought to my attention by:

> Suggested-by: Joseph Myers <joseph@codesourcery.com>

> 

> Signed-off-by: Paul A. Clarke <pc@us.ibm.com>


We do not use sign-off.

LGTM.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>


> ---

>  sysdeps/ieee754/flt-32/e_powf.c      | 4 ++--

>  sysdeps/ieee754/flt-32/math_config.h | 9 ---------

>  2 files changed, 2 insertions(+), 11 deletions(-)

> 

> diff --git a/sysdeps/ieee754/flt-32/e_powf.c b/sysdeps/ieee754/flt-32/e_powf.c

> index 4947ae2..d4f52f2 100644

> --- a/sysdeps/ieee754/flt-32/e_powf.c

> +++ b/sysdeps/ieee754/flt-32/e_powf.c

> @@ -158,9 +158,9 @@ __powf (float x, float y)

>        if (__glibc_unlikely (zeroinfnan (iy)))

>  	{

>  	  if (2 * iy == 0)

> -	    return issignalingf_inline (x) ? x + y : 1.0f;

> +	    return __issignalingf (x) ? x + y : 1.0f;

>  	  if (ix == 0x3f800000)

> -	    return issignalingf_inline (y) ? x + y : 1.0f;

> +	    return __issignalingf (y) ? x + y : 1.0f;

>  	  if (2 * ix > 2u * 0x7f800000 || 2 * iy > 2u * 0x7f800000)

>  	    return x + y;

>  	  if (2 * ix == 2 * 0x3f800000)

> diff --git a/sysdeps/ieee754/flt-32/math_config.h b/sysdeps/ieee754/flt-32/math_config.h

> index c5e9299..57cf441 100644

> --- a/sysdeps/ieee754/flt-32/math_config.h

> +++ b/sysdeps/ieee754/flt-32/math_config.h

> @@ -101,15 +101,6 @@ asdouble (uint64_t i)

>    return u.f;

>  }

>  

> -static inline int

> -issignalingf_inline (float x)

> -{

> -  uint32_t ix = asuint (x);

> -  if (HIGH_ORDER_BIT_IS_SET_FOR_SNAN)

> -    return (ix & 0x7fc00000) == 0x7fc00000;

> -  return 2 * (ix ^ 0x00400000) > 2u * 0x7fc00000;

> -}

> -

>  #define NOINLINE __attribute__ ((noinline))

>  

>  attribute_hidden float __math_oflowf (uint32_t);

>
Joseph Myers Nov. 21, 2019, 8:59 p.m. | #2
On Thu, 21 Nov 2019, Paul A. Clarke wrote:

> From: "Paul A. Clarke" <pc@us.ibm.com>

> 

> Very recent commit 854e91bf6b4221f424ffa13b9ef50f35623b7b74 enabled

> inline of issignalingf() in general (__issignalingf in include/math.h).

> There is another implementation for an inline use of issignalingf

> (issignalingf_inline in sysdeps/ieee754/flt-32/math_config.h)

> which could instead make use of the new enablement.

> 

> Replace the use of issignalingf_inline with __issignaling.


I think it's better to replace it with calls to issignaling (the 
type-generic macro), rather than to __issignalingf (the inline function).  
That way, if in future GCC gets __builtin_issignaling and glibc's <math.h> 
starts to use it, this code will automatically start using 
__builtin_issignaling via the macro definition.

-- 
Joseph S. Myers
joseph@codesourcery.com

Patch

diff --git a/sysdeps/ieee754/flt-32/e_powf.c b/sysdeps/ieee754/flt-32/e_powf.c
index 4947ae2..d4f52f2 100644
--- a/sysdeps/ieee754/flt-32/e_powf.c
+++ b/sysdeps/ieee754/flt-32/e_powf.c
@@ -158,9 +158,9 @@  __powf (float x, float y)
       if (__glibc_unlikely (zeroinfnan (iy)))
 	{
 	  if (2 * iy == 0)
-	    return issignalingf_inline (x) ? x + y : 1.0f;
+	    return __issignalingf (x) ? x + y : 1.0f;
 	  if (ix == 0x3f800000)
-	    return issignalingf_inline (y) ? x + y : 1.0f;
+	    return __issignalingf (y) ? x + y : 1.0f;
 	  if (2 * ix > 2u * 0x7f800000 || 2 * iy > 2u * 0x7f800000)
 	    return x + y;
 	  if (2 * ix == 2 * 0x3f800000)
diff --git a/sysdeps/ieee754/flt-32/math_config.h b/sysdeps/ieee754/flt-32/math_config.h
index c5e9299..57cf441 100644
--- a/sysdeps/ieee754/flt-32/math_config.h
+++ b/sysdeps/ieee754/flt-32/math_config.h
@@ -101,15 +101,6 @@  asdouble (uint64_t i)
   return u.f;
 }
 
-static inline int
-issignalingf_inline (float x)
-{
-  uint32_t ix = asuint (x);
-  if (HIGH_ORDER_BIT_IS_SET_FOR_SNAN)
-    return (ix & 0x7fc00000) == 0x7fc00000;
-  return 2 * (ix ^ 0x00400000) > 2u * 0x7fc00000;
-}
-
 #define NOINLINE __attribute__ ((noinline))
 
 attribute_hidden float __math_oflowf (uint32_t);