diff mbox

[v2] Enable inlining issignalingf within glibc

Message ID 1569619615-11714-1-git-send-email-pc@us.ibm.com
State Superseded
Delegated to: Joseph Myers
Headers

Commit Message

Paul A. Clarke Sept. 27, 2019, 9:26 p.m. UTC 
  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.

Establish a means to inline issignalingf, and use that for powerpc.

2019-09-25  Paul A. Clarke  <pc@us.ibm.com>

	* math/s_canonicalize_template.c: Include math_private.h
	* math/s_fmin_template.c: Likewise.
	* math/s_fmax_template.c: Likewise.
	* math/s_fmaxmag_template.c: Likewise.
	* math/s_fminmag_template.c: Likewise.
	* sysdeps/ieee754/flt-32/s_issignalingf.c: Move code body
	to __issignalingf_inline. Undefine __issignalingf.
	Include s_issignalingf_inline.h.
	* sysdeps/ieee754/flt-32/s_issignalingf_inline.h: New.
	* sysdeps/powerpc/fpu/math_private.h: (__issignalingf): New.
	Include s_issignalingf_inline.h.
---
v2: Oops. Forgot to include <s_issignalingf_inline.h> in s_signalingf.c
  so other arches will continue to work as before.

 math/s_canonicalize_template.c                 |  1 +
 math/s_fmax_template.c                         |  1 +
 math/s_fmaxmag_template.c                      |  1 +
 math/s_fmin_template.c                         |  2 +-
 math/s_fminmag_template.c                      |  1 +
 sysdeps/ieee754/flt-32/s_issignalingf.c        | 20 +++--------
 sysdeps/ieee754/flt-32/s_issignalingf_inline.h | 47 ++++++++++++++++++++++++++
 sysdeps/powerpc/fpu/math_private.h             |  3 ++
 8 files changed, 59 insertions(+), 17 deletions(-)
 create mode 100644 sysdeps/ieee754/flt-32/s_issignalingf_inline.h

Comments

Joseph Myers Sept. 27, 2019, 9:46 p.m. UTC | #1 
On Fri, 27 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.
> 
> Establish a means to inline issignalingf, and use that for powerpc.

Please don't add any math_private.h includes for optimizing function 
calls, or any optimized definitions of math.h functions that only live in 
math_private.h.  (Optimized definitions of functions that are only 
declared in math_private.h, such as __ieee754_* functions, are OK.)

See my commit c52944e8ccb15158b7e44cbb75fb46d81400d75c.  Where code uses 
anything from math_private.h, we want it to be something *only* in 
math_private.h, so that a build failure results if a math_private.h 
include is missing, rather than silently having less optimized code.

I.e. any such optimized inlines belong in include/math.h like the one for 
__isinff128 for GCC before GCC 7.
diff mbox

Patch

diff --git a/math/s_canonicalize_template.c b/math/s_canonicalize_template.c
index 81a74a7..4689963 100644
--- a/math/s_canonicalize_template.c
+++ b/math/s_canonicalize_template.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <math.h>
+#include <math_private.h>
 
 int
 M_DECL_FUNC (__canonicalize) (FLOAT *cx, const FLOAT *x)
diff --git a/math/s_fmax_template.c b/math/s_fmax_template.c
index 098c1e5..394c418 100644
--- a/math/s_fmax_template.c
+++ b/math/s_fmax_template.c
@@ -18,6 +18,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <math.h>
+#include <math_private.h>
 
 FLOAT
 M_DECL_FUNC (__fmax) (FLOAT x, FLOAT y)
diff --git a/math/s_fmaxmag_template.c b/math/s_fmaxmag_template.c
index 1ebe4cc..41f33aa 100644
--- a/math/s_fmaxmag_template.c
+++ b/math/s_fmaxmag_template.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <math.h>
+#include <math_private.h>
 
 FLOAT
 M_DECL_FUNC (__fmaxmag) (FLOAT x, FLOAT y)
diff --git a/math/s_fmin_template.c b/math/s_fmin_template.c
index 259e280..63edcaa 100644
--- a/math/s_fmin_template.c
+++ b/math/s_fmin_template.c
@@ -18,7 +18,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <math.h>
-
+#include <math_private.h>
 
 FLOAT
 M_DECL_FUNC (__fmin) (FLOAT x, FLOAT y)
diff --git a/math/s_fminmag_template.c b/math/s_fminmag_template.c
index 8f1b3d4..4a7a3ba 100644
--- a/math/s_fminmag_template.c
+++ b/math/s_fminmag_template.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <math.h>
+#include <math_private.h>
 
 FLOAT
 M_DECL_FUNC (__fminmag) (FLOAT x, FLOAT y)
diff --git a/sysdeps/ieee754/flt-32/s_issignalingf.c b/sysdeps/ieee754/flt-32/s_issignalingf.c
index 50da641..5532474 100644
--- a/sysdeps/ieee754/flt-32/s_issignalingf.c
+++ b/sysdeps/ieee754/flt-32/s_issignalingf.c
@@ -19,25 +19,13 @@ 
 #include <math.h>
 #include <math_private.h>
 #include <nan-high-order-bit.h>
+#include <s_issignalingf_inline.h>
+
+#undef __issignalingf
 
 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
+  return __issignalingf_inline (x);
 }
 libm_hidden_def (__issignalingf)
diff --git a/sysdeps/ieee754/flt-32/s_issignalingf_inline.h b/sysdeps/ieee754/flt-32/s_issignalingf_inline.h
new file mode 100644
index 0000000..130eace
--- /dev/null
+++ b/sysdeps/ieee754/flt-32/s_issignalingf_inline.h
@@ -0,0 +1,47 @@ 
+/* Test for signaling NaN.
+   Copyright (C) 2013-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _S_ISSIGNALING_INLINE_H
+#define  _S_ISSIGNALING_INLINE_H
+
+#include <math.h>
+#include <math_private.h>
+#include <nan-high-order-bit.h>
+
+extern __always_inline int
+__issignalingf_inline (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
+}
+
+#endif
diff --git a/sysdeps/powerpc/fpu/math_private.h b/sysdeps/powerpc/fpu/math_private.h
index 9b9391a..809d77e 100644
--- a/sysdeps/powerpc/fpu/math_private.h
+++ b/sysdeps/powerpc/fpu/math_private.h
@@ -35,4 +35,7 @@  __ieee754_sqrtf128 (_Float128 __x)
 }
 #endif
 
+#define __issignalingf __issignalingf_inline
+#include <s_issignalingf_inline.h>
+
 #endif /* _PPC_MATH_PRIVATE_H_ */