diff mbox

Remove ancient __signbit inlines

Message ID DB6PR0801MB205367C531C71D4E40A0023F837B0@DB6PR0801MB2053.eurprd08.prod.outlook.com
State New, archived
Headers show

Commit Message

Wilco Dijkstra Sept. 26, 2017, 10:59 a.m. UTC
Remove __signbit inlines from mathinline.h.  Math.h already uses
the builtin when supported, so additional inlines are only used
on pre 4.0 GCCs.  Similarly remove a few old copysign and fabs
inlines.

OK for commit?

ChangeLog:
2017-09-25  Wilco Dijkstra  <wdijkstr@arm.com>

	* sysdeps/alpha/fpu/bits/mathinline.h (__inline_copysign): Remove.
	(__inline_fabs): Remove.
	(__signbitf): Remove.
	(__signbit): Remove.
	(__signbitl): Remove.
	* sysdeps/ia64/fpu/bits/mathinline.h: Delete file.
	* sysdeps/m68k/coldfire/fpu/bits/mathinline.h: Delete file.
	* sysdeps/m68k/m680x0/fpu/bits/mathinline.h: (__signbitf): Remove.
	(__signbit): Remove.
	(__signbitl): Remove.
	* sysdeps/powerpc/bits/mathinline.h (__signbitf): Remove.
	(__signbit): Remove.
	(__signbitl): Remove.
	* sysdeps/s390/fpu/bits/mathinline.h: Delete file.
	* sysdeps/sparc/fpu/bits/mathinline.h (__signbitf): Remove.
	(__signbit): Remove.
	(__signbitl): Remove.
	* sysdeps/tile/bits/mathinline.h: Delete file.
	* sysdeps/x86/fpu/bits/mathinline.h (__signbitf): Remove.
	(__signbit): Remove.
	(__signbitl): Remove.

--

Comments

Joseph Myers Sept. 26, 2017, 5:17 p.m. UTC | #1
On Tue, 26 Sep 2017, Wilco Dijkstra wrote:

> Remove __signbit inlines from mathinline.h.  Math.h already uses
> the builtin when supported, so additional inlines are only used
> on pre 4.0 GCCs.  Similarly remove a few old copysign and fabs
> inlines.

When submitting a patch, please always include a description of how it was 
tested.

In this case, I'd expect compilation testing for at least one 
configuration using each header that is changed (except ColdFire which 
lacks a working glibc build at present), with before-and-after comparison 
of stripped binaries and an explanation if the stripped binaries differed 
in any way.

The patch is OK if it's passed such testing with unchanged stripped 
binaries.

Note that the copysign/fabs inlines removed include ones for __copysign / 
__fabs functions which are *not* inlined by the compiler automatically.  
The removal is still OK given that those inlines wouldn't have been active 
with any compiler version supported for building glibc.  But glibc does 
have internal __fabs* calls that it would make sense to change to call the 
public functions directly so __fabs* inlines are never necessary (and 
internal __copysign* calls that it would make sense to change to call 
copysign* directly so that the inline in 
sysdeps/generic/math_private_calls.h can be removed).  (Changes to call 
fabsl in ldbl-128ibm would need -fno-builtin-fabsl additions to 
sysdeps/powerpc/nofpu/Makefile and both such changes would need powerpc32 
soft-float / e500v1 compilation testing to make sure no ldbl-128ibm 
functions get called internally by double functions resulting in namespace 
issues.  Care would also be needed not to get localplt issues for fabsf128 
or copysignf128 in configurations using float128, as only 
__builtin_fabsf128 / __builtin_copysignf128 get inlined by the compiler.)
Wilco Dijkstra Sept. 27, 2017, 12:32 p.m. UTC | #2
Joseph Myers wrote:

> When submitting a patch, please always include a description of how it was 
> tested.

So far I've tested it on AArch64. This particular patch is trivially proven to
not affect the GLIBC binaries due to (a) only removing code guarded by
__GNUC_PREREQ with an older version than the minimum required for
building GLIBC, and (b) removing inlines that are not used inside GLIBC.

For the __fabs and __copysign inlines (a) applies.
For the __signbit inlines (b) applies, and in many cases (a) as well.

I'll try setting up the build many GLIBC script to ensure there is no trivial edit
mistake like unmatched #endif. 

> Note that the copysign/fabs inlines removed include ones for __copysign / 
> __fabs functions which are *not* inlined by the compiler automatically.

Yes I've noticed there are still a few calls to __fabs, I'll fix those in a seperate
path. Any remaining __fabs inlines can be removed too (I'll try to avoid 
float128 changes as it appears quite complex).

Renaming __copysign is possible too, but given that one is already inlined,
it seems best to first enable inlining for all simple math functions (round, rint,
ceil etc). I'll do this like sqrt so it works on all targets without requiring target
specific inlines. Hopefully we can get rid of all target math inlines this release!

Wilco
diff mbox

Patch

diff --git a/sysdeps/alpha/fpu/bits/mathinline.h b/sysdeps/alpha/fpu/bits/mathinline.h
index 00c8c42a83bc904c4c788db57753ff95b299f3c8..ceb8a4416c7b1d8372cd2048893536966de8ead1 100644
--- a/sysdeps/alpha/fpu/bits/mathinline.h
+++ b/sysdeps/alpha/fpu/bits/mathinline.h
@@ -42,84 +42,4 @@ 
       __r != 0; }))
 #endif /* ISO C99 */
 
-#if (!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \
-    && defined __OPTIMIZE__
-
-#if !__GNUC_PREREQ (4, 0)
-# define __inline_copysign(NAME, TYPE)					\
-__MATH_INLINE TYPE							\
-__NTH (NAME (TYPE __x, TYPE __y))					\
-{									\
-  TYPE __z;								\
-  __asm ("cpys %1, %2, %0" : "=f" (__z) : "f" (__y), "f" (__x));	\
-  return __z;								\
-}
-
-__inline_copysign (__copysignf, float)
-__inline_copysign (copysignf, float)
-__inline_copysign (__copysign, double)
-__inline_copysign (copysign, double)
-
-# undef __inline_copysign
-#endif
-
-
-#if !__GNUC_PREREQ (2, 8)
-# define __inline_fabs(NAME, TYPE)			\
-__MATH_INLINE TYPE					\
-__NTH (NAME (TYPE __x))					\
-{							\
-  TYPE __z;						\
-  __asm ("cpys $f31, %1, %0" : "=f" (__z) : "f" (__x));	\
-  return __z;						\
-}
-
-__inline_fabs (__fabsf, float)
-__inline_fabs (fabsf, float)
-__inline_fabs (__fabs, double)
-__inline_fabs (fabs, double)
-
-# undef __inline_fabs
-#endif
-
-#ifdef __USE_ISOC99
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-#if !__GNUC_PREREQ (4, 0)
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-#else
-  return __builtin_signbitf (__x);
-#endif
-}
-
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-#if !__GNUC_PREREQ (4, 0)
-  __extension__ union { double __d; long __i; } __u = { __d: __x };
-  return __u.__i < 0;
-#else
-  return __builtin_signbit (__x);
-#endif
-}
-
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-#if !__GNUC_PREREQ (4, 0)
-  __extension__ union {
-    long double __d;
-    long __i[sizeof(long double)/sizeof(long)];
-  } __u = { __d: __x };
-  return __u.__i[sizeof(long double)/sizeof(long) - 1] < 0;
-#else
-  return __builtin_signbitl (__x);
-#endif
-}
-#endif /* C99 */
-
 #endif /* __NO_MATH_INLINES */
diff --git a/sysdeps/ia64/fpu/bits/mathinline.h b/sysdeps/ia64/fpu/bits/mathinline.h
deleted file mode 100644
index c2107af275ef336995d973c9e0aa96cd75029fc3..0000000000000000000000000000000000000000
--- a/sysdeps/ia64/fpu/bits/mathinline.h
+++ /dev/null
@@ -1,53 +0,0 @@ 
-/* Inline math functions for ia64.
-   Copyright (C) 2004-2017 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
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _MATH_H
-# error "Never use <bits/mathinline.h> directly; include <math.h> instead."
-#endif
-
-#ifndef __extern_inline
-# define __MATH_INLINE __inline
-#else
-# define __MATH_INLINE __extern_inline
-#endif
-
-#if defined __USE_ISOC99 && defined __GNUC__ && __GNUC__ >= 2
-/* The gcc, version 2.7 or below, has problems with all this inlining
-   code.  So disable it for this version of the compiler.  */
-# if __GNUC_PREREQ (2, 8)
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-}
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  __extension__ union { double __d; int __i[2]; } __u = { __d: __x };
-  return __u.__i[1] < 0;
-}
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __l; int __i[3]; } __u = { __l: __x };
-  return (__u.__i[2] & 0x8000) != 0;
-}
-# endif
-#endif
diff --git a/sysdeps/m68k/coldfire/fpu/bits/mathinline.h b/sysdeps/m68k/coldfire/fpu/bits/mathinline.h
deleted file mode 100644
index 7dbfe37ee1efddcf4994ab1b94a06e620bbe01cd..0000000000000000000000000000000000000000
--- a/sysdeps/m68k/coldfire/fpu/bits/mathinline.h
+++ /dev/null
@@ -1,44 +0,0 @@ 
-/* Inline math functions for Coldfire.
-   Copyright (C) 2012-2017 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
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _MATH_H
-# error "Never use <bits/mathinline.h> directly; include <math.h> instead."
-#endif
-
-#ifndef __extern_always_inline
-# define __MATH_INLINE __inline
-#else
-# define __MATH_INLINE __extern_always_inline
-#endif
-
-#if defined __USE_ISOC99 && defined __GNUC__
-
-/* Test for negative number.  Used in the signbit macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  return __builtin_signbitf (__x);
-}
-
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  return __builtin_signbit (__x);
-}
-
-#endif
diff --git a/sysdeps/m68k/m680x0/fpu/bits/mathinline.h b/sysdeps/m68k/m680x0/fpu/bits/mathinline.h
index b92b1f83e7f6f39698ad8501bb2dec40d02c29ba..759c83d2b14323b13eccbfc02c15298fd875257c 100644
--- a/sysdeps/m68k/m680x0/fpu/bits/mathinline.h
+++ b/sysdeps/m68k/m680x0/fpu/bits/mathinline.h
@@ -83,26 +83,6 @@ 
 	       : "=dm" (__result) : "f" (x), "f" (y));	\
       __result != 0; })
 # endif /* GCC 3.1 */
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-}
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  __extension__ union { double __d; int __i[2]; } __u = { __d: __x };
-  return __u.__i[0] < 0;
-}
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __d; int __i[3]; } __u = { __d: __x };
-  return __u.__i[0] < 0;
-}
 #endif
 
 
diff --git a/sysdeps/powerpc/bits/mathinline.h b/sysdeps/powerpc/bits/mathinline.h
index e5f0cd30f2598f9ad78fdb079713d742251b9139..3c297899c50bb148059e6339db9c68d8da93229c 100644
--- a/sysdeps/powerpc/bits/mathinline.h
+++ b/sysdeps/powerpc/bits/mathinline.h
@@ -54,38 +54,6 @@ 
 
 # endif /* __GNUC_PREREQ (2,97) */
 
-/* The gcc, version 2.7 or below, has problems with all this inlining
-   code.  So disable it for this version of the compiler.  */
-# if __GNUC_PREREQ (2, 8)
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-#if __GNUC_PREREQ (4, 0)
-  return __builtin_signbitf (__x);
-#else
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-#endif
-}
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-#if __GNUC_PREREQ (4, 0)
-  return __builtin_signbit (__x);
-#else
-  __extension__ union { double __d; long long __i; } __u = { __d: __x };
-  return __u.__i < 0;
-#endif
-}
-#  ifdef __LONG_DOUBLE_128__
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  return __signbit ((double) __x);
-}
-#  endif
-# endif
 #endif /* __USE_ISOC99 */
 
 #if !defined __NO_MATH_INLINES && defined __OPTIMIZE__
diff --git a/sysdeps/s390/fpu/bits/mathinline.h b/sysdeps/s390/fpu/bits/mathinline.h
deleted file mode 100644
index 76b3be50c4e347281ed7790aa5eafbdb36d95052..0000000000000000000000000000000000000000
--- a/sysdeps/s390/fpu/bits/mathinline.h
+++ /dev/null
@@ -1,66 +0,0 @@ 
-/* Inline math functions for s390.
-   Copyright (C) 2004-2017 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
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _MATH_H
-# error "Never use <bits/mathinline.h> directly; include <math.h> instead."
-#endif
-
-#ifndef __extern_inline
-# define __MATH_INLINE __inline
-#else
-# define __MATH_INLINE __extern_inline
-#endif
-
-#if (!defined __NO_MATH_INLINES || defined __LIBC_INTERNAL_MATH_INLINES) \
-    && defined __OPTIMIZE__
-
-#ifdef __USE_ISOC99
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-}
-
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  __extension__ union { double __d; long __i; } __u = { __d: __x };
-  return __u.__i < 0;
-}
-
-# ifndef __NO_LONG_DOUBLE_MATH
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __l; int __i[4]; } __u = { __l: __x };
-  return __u.__i[0] < 0;
-}
-# else
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  return __signbit ((double) __x);
-}
-# endif
-
-#endif /* C99 */
-
-#endif /* __NO_MATH_INLINES */
diff --git a/sysdeps/sparc/fpu/bits/mathinline.h b/sysdeps/sparc/fpu/bits/mathinline.h
index 2939af5bc623287bab02dab72d664509d435f2f9..9b2847b6a882c2eb029c835c1e34101257240928 100644
--- a/sysdeps/sparc/fpu/bits/mathinline.h
+++ b/sysdeps/sparc/fpu/bits/mathinline.h
@@ -132,67 +132,6 @@ 
 #  define __MATH_INLINE __extern_inline
 # endif  /* __cplusplus */
 
-/* The gcc, version 2.7 or below, has problems with all this inlining
-   code.  So disable it for this version of the compiler.  */
-# if __GNUC_PREREQ (2, 8)
-
-#  ifdef __USE_ISOC99
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-}
-
-#   if __WORDSIZE == 32
-
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  __extension__ union { double __d; int __i[2]; } __u = { __d: __x };
-  return __u.__i[0] < 0;
-}
-
-#    ifndef __NO_LONG_DOUBLE_MATH
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __l; int __i[4]; } __u = { __l: __x };
-  return __u.__i[0] < 0;
-}
-#    else
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  return __signbit ((double)__x);
-}
-#    endif
-
-#   else /* sparc64 */
-
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  __extension__ union { double __d; long int __i; } __u = { __d: __x };
-  return __u.__i < 0;
-}
-
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __l; long int __i[2]; } __u = { __l: __x };
-  return __u.__i[0] < 0;
-}
-
-#   endif /* sparc64 */
-
-#  endif /* __USE_ISOC99 */
-
-/* This code is used internally in the GNU libc.  */
-# endif /* gcc 2.8+ */
-
 # ifdef __USE_ISOC99
 
 #  ifndef __NO_MATH_INLINES
diff --git a/sysdeps/tile/bits/mathinline.h b/sysdeps/tile/bits/mathinline.h
deleted file mode 100644
index 5692d91004c28891b81fc5021d604f0c9aec35d9..0000000000000000000000000000000000000000
--- a/sysdeps/tile/bits/mathinline.h
+++ /dev/null
@@ -1,44 +0,0 @@ 
-/* Copyright (C) 2011-2017 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Chris Metcalf <cmetcalf@tilera.com>, 2011.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-#ifndef _MATH_H
-# error "Never use <bits/mathinline.h> directly; include <math.h> instead."
-#endif
-
-#ifndef __extern_always_inline
-# define __MATH_INLINE __inline
-#else
-# define __MATH_INLINE __extern_always_inline
-#endif
-
-
-#if defined __USE_ISOC99 && defined __GNUC__
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-  return __builtin_signbitf (__x);
-}
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-  return __builtin_signbit (__x);
-}
-
-#endif
diff --git a/sysdeps/x86/fpu/bits/mathinline.h b/sysdeps/x86/fpu/bits/mathinline.h
index a183693ae6978a875f80bfd61a1fa5e5333de204..41250b452c7c291537a7ba3718b55f58c90a4600 100644
--- a/sysdeps/x86/fpu/bits/mathinline.h
+++ b/sysdeps/x86/fpu/bits/mathinline.h
@@ -117,43 +117,6 @@ 
 #  endif /* __i686__ */
 # endif	/* GCC 2.97 */
 
-/* The gcc, version 2.7 or below, has problems with all this inlining
-   code.  So disable it for this version of the compiler.  */
-# if __GNUC_PREREQ (2, 8)
-
-/* Test for negative number.  Used in the signbit() macro.  */
-__MATH_INLINE int
-__NTH (__signbitf (float __x))
-{
-#  ifdef __SSE2_MATH__
-  int __m;
-  __asm ("pmovmskb %1, %0" : "=r" (__m) : "x" (__x));
-  return (__m & 0x8) != 0;
-#  else
-  __extension__ union { float __f; int __i; } __u = { __f: __x };
-  return __u.__i < 0;
-#  endif
-}
-__MATH_INLINE int
-__NTH (__signbit (double __x))
-{
-#  ifdef __SSE2_MATH__
-  int __m;
-  __asm ("pmovmskb %1, %0" : "=r" (__m) : "x" (__x));
-  return (__m & 0x80) != 0;
-#  else
-  __extension__ union { double __d; int __i[2]; } __u = { __d: __x };
-  return __u.__i[1] < 0;
-#  endif
-}
-__MATH_INLINE int
-__NTH (__signbitl (long double __x))
-{
-  __extension__ union { long double __l; int __i[3]; } __u = { __l: __x };
-  return (__u.__i[2] & 0x8000) != 0;
-}
-
-# endif
 #endif