[v4,3/3] Cleanup __ieee754_sqrt(f/l)

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

Commit Message

Wilco Dijkstra March 15, 2018, 3:56 p.m. UTC
  Finally remove the now unused target specific__ieee754_sqrt(f/l) inlines.

ChangeLog:
2018-03-15  Wilco Dijkstra  <wdijkstr@arm.com>

	* sysdeps/aarch64/fpu/math_private.h (__ieee754_sqrt): Remove.
	(__ieee754_sqrtf): Remove.
	* sysdeps/alpha/fpu/math_private.h (__ieee754_sqrt): Remove.
	(__ieee754_sqrtf): Remove.
	* sysdeps/generic/math-type-macros.h (M_SQRT): Use __builtin_sqrt(f).
	* sysdeps/m68k/m680x0/fpu/mathimpl.h (__ieee754_sqrt): Remove.
	* sysdeps/powerpc/fpu/math_private.h (__ieee754_sqrt): Remove.
	(__ieee754_sqrtf): Remove.
	* sysdeps/s390/fpu/bits/mathinline.h: Remove file.
	* sysdeps/sparc/fpu/bits/mathinline.h (sqrt) Remove.
	(sqrtf): Remove.
	(sqrtl): Remove.
	(__ieee754_sqrt): Remove.
	(__ieee754_sqrtf): Remove.
	(__ieee754_sqrtl): Remove.
	* sysdeps/m68k/m680x0/fpu/mathimpl.h (__ieee754_sqrt): Remove.
	* sysdeps/x86/fpu/math_private.h (__ieee754_sqrt): Remove.
	* sysdeps/x86_64/fpu/math_private.h (__ieee754_sqrt): Remove.
	(__ieee754_sqrtf): Remove.
	(__ieee754_sqrtl): Remove.

--
  

Comments

Joseph Myers March 15, 2018, 5:13 p.m. UTC | #1
On Thu, 15 Mar 2018, Wilco Dijkstra wrote:

> Finally remove the now unused target specific__ieee754_sqrt(f/l) inlines.

OK.  To be clear:

> 	* sysdeps/generic/math-type-macros.h (M_SQRT): Use __builtin_sqrt(f).

This ChangeLog entry isn't correct, it's sqrt not __builtin_sqrt that is 
used after the patch.  (And actually this particular change is logically 
like the ones in patch 2 - it's one of the changes that causes 
__ieee754_sqrt to be unused by arranging for direct use of sqrt, rather 
than one removing an inline that is now unused.)

Also, for the commit message: the removals from 
sysdeps/sparc/fpu/bits/mathinline.h include removals of 
user-visible-for-old-GCC sqrt inlines.  Removing those is desirable, under 
the general principle of leaving such inlining to the compiler rather than 
trying to do it in installed headers, especially when only very old 
compilers are affected, but it's logically separate from changing how 
things are built inside glibc.

Also: I believe this change would slightly pessimize calls to the 
out-of-line sqrt function wrappers in the case where __ieee754_sqrt would 
previously have been inlined in those wrappers but now those inlines are 
being removed.  I consider that explicitly OK, because on such 
architectures the compiler should have inlined sqrt calls (even with 
-fmath-error, GCC generates code that uses the sqrt instruction and only 
calls the out-of-line function when needed to set errno - so on such 
architectures, the out-of-line function should normally never be called 
except for negative input, which is not what we care about optimizing).
  
Joseph Myers March 16, 2018, 12:21 a.m. UTC | #2
This appears to have broken the build of sqrtl for m68k, at least with GCC 
6.

https://sourceware.org/ml/libc-testresults/2018-q1/msg00475.html

In file included from ../include/sys/cdefs.h:3:0,
                 from ../include/features.h:428,
                 from ../bits/libc-header-start.h:33,
                 from ../math/math.h:27,
                 from ../include/math.h:7,
                 from ../sysdeps/m68k/m680x0/fpu/e_acos.c:18,
                 from ../sysdeps/m68k/m680x0/fpu/e_acosl.c:6,
                 from ../sysdeps/m68k/m680x0/fpu/e_sqrtl.c:3:
../sysdeps/m68k/m680x0/fpu/e_acos.c: In function '__ieee754_sqrtl':
../sysdeps/m68k/m680x0/fpu/bits/mathinline.h:39:31: error: implicit declaration of function '____ieee754_sqrtl' [-Werror=implicit-function-declaration]
 # define __m81_u(x)  __CONCAT(__,x)
                               ^
../misc/sys/cdefs.h:105:23: note: in definition of macro '__CONCAT'
 #define __CONCAT(x,y) x ## y
                       ^
../sysdeps/m68k/m680x0/fpu/e_acos.c:33:10: note: in expansion of macro 
'__m81_u'
   return __m81_u(FUNC)(x);
          ^~~~~~~
  

Patch

diff --git a/sysdeps/aarch64/fpu/math_private.h b/sysdeps/aarch64/fpu/math_private.h
index 3eea86eac19520a0932c6debd2a616f035fcf5f1..d9c2d710a95ebe76b25c9dc86ed0bea729a3a6cd 100644
--- a/sysdeps/aarch64/fpu/math_private.h
+++ b/sysdeps/aarch64/fpu/math_private.h
@@ -27,22 +27,6 @@ 
 #define math_force_eval(x) \
 ({ __typeof (x) __x = (x); __asm __volatile__ ("" : : "w" (__x)); })
 
-extern __always_inline double
-__ieee754_sqrt (double d)
-{
-  double res;
-  asm __volatile__ ("fsqrt   %d0, %d1" : "=w" (res) : "w" (d));
-  return res;
-}
-
-extern __always_inline float
-__ieee754_sqrtf (float s)
-{
-  float res;
-  asm __volatile__ ("fsqrt   %s0, %s1" : "=w" (res) : "w" (s));
-  return res;
-}
-
 static __always_inline void
 libc_feholdexcept_aarch64 (fenv_t *envp)
 {
diff --git a/sysdeps/alpha/fpu/math_private.h b/sysdeps/alpha/fpu/math_private.h
index 1e97c867c38767a2237df21f84e9c55a53ad13bb..95dc32c969c1133add0d5a1c42e1f1bd0c9d0e2d 100644
--- a/sysdeps/alpha/fpu/math_private.h
+++ b/sysdeps/alpha/fpu/math_private.h
@@ -21,30 +21,4 @@ 
 
 #include_next <math_private.h>
 
-#ifdef __alpha_fix__
-extern __always_inline double
-__ieee754_sqrt (double d)
-{
-  double ret;
-# ifdef _IEEE_FP_INEXACT
-  asm ("sqrtt/suid %1,%0" : "=&f"(ret) : "f"(d));
-# else
-  asm ("sqrtt/sud %1,%0" : "=&f"(ret) : "f"(d));
-# endif
-  return ret;
-}
-
-extern __always_inline float
-__ieee754_sqrtf (float d)
-{
-  float ret;
-# ifdef _IEEE_FP_INEXACT
-  asm ("sqrts/suid %1,%0" : "=&f"(ret) : "f"(d));
-# else
-  asm ("sqrts/sud %1,%0" : "=&f"(ret) : "f"(d));
-# endif
-  return ret;
-}
-#endif /* FIX */
-
 #endif /* ALPHA_MATH_PRIVATE_H */
diff --git a/sysdeps/generic/math-type-macros.h b/sysdeps/generic/math-type-macros.h
index 8ceaa4e02fc3a56d7fae56111b93dd7e44d050a3..ffcf7b80b608df537f578fcb0ae584d46fe68eb1 100644
--- a/sysdeps/generic/math-type-macros.h
+++ b/sysdeps/generic/math-type-macros.h
@@ -91,7 +91,7 @@ 
 #define M_HYPOT M_SUF (__ieee754_hypot)
 #define M_LOG M_SUF (__ieee754_log)
 #define M_SINH M_SUF (__ieee754_sinh)
-#define M_SQRT M_SUF (__ieee754_sqrt)
+#define M_SQRT M_SUF (sqrt)
 
 /* Needed to evaluate M_MANT_DIG below.  */
 #include <float.h>
diff --git a/sysdeps/m68k/m680x0/fpu/mathimpl.h b/sysdeps/m68k/m680x0/fpu/mathimpl.h
index 848f9cffaadad09443cd539ceb68da5712a93b39..d39094cccbd3712dfae4601967ee695722b446a0 100644
--- a/sysdeps/m68k/m680x0/fpu/mathimpl.h
+++ b/sysdeps/m68k/m680x0/fpu/mathimpl.h
@@ -30,7 +30,6 @@  __inline_mathop	(__ieee754_exp10, tentox,)
 __inline_mathop	(__ieee754_log10, log10,)
 __inline_mathop	(__ieee754_log2, log2,)
 __inline_mathop	(__ieee754_log, logn,)
-__inline_mathop	(__ieee754_sqrt, sqrt,)
 __inline_mathop	(__ieee754_atanh, atanh,)
 
 __m81_defun (double, __ieee754_remainder, (double __x, double __y),)
diff --git a/sysdeps/powerpc/fpu/math_private.h b/sysdeps/powerpc/fpu/math_private.h
index 4d1e571f8489300c35ca8a44cd0d83c5199f2267..b9fc721257235a549873beb9e31fa30085b76d4a 100644
--- a/sysdeps/powerpc/fpu/math_private.h
+++ b/sysdeps/powerpc/fpu/math_private.h
@@ -42,36 +42,6 @@  __ieee754_sqrtf128 (_Float128 __x)
 }
 #endif
 
-extern double __slow_ieee754_sqrt (double);
-extern __always_inline double
-__ieee754_sqrt (double __x)
-{
-  double __z;
-
-#ifdef _ARCH_PPCSQ
-   asm ("fsqrt	%0,%1" : "=f" (__z) : "f" (__x));
-#else
-   __z = __slow_ieee754_sqrt(__x);
-#endif
-
-  return __z;
-}
-
-extern float __slow_ieee754_sqrtf (float);
-extern __always_inline float
-__ieee754_sqrtf (float __x)
-{
-  float __z;
-
-#ifdef _ARCH_PPCSQ
-  asm ("fsqrts	%0,%1" : "=f" (__z) : "f" (__x));
-#else
-   __z = __slow_ieee754_sqrtf(__x);
-#endif
-
-  return __z;
-}
-
 #if defined _ARCH_PWR5X
 
 # ifndef __round
diff --git a/sysdeps/s390/fpu/bits/mathinline.h b/sysdeps/s390/fpu/bits/mathinline.h
deleted file mode 100644
index 83ce6fa4e0915aaa7651263255776e96d9aee290..0000000000000000000000000000000000000000
--- a/sysdeps/s390/fpu/bits/mathinline.h
+++ /dev/null
@@ -1,66 +0,0 @@ 
-/* Inline math functions for s390.
-   Copyright (C) 2004-2018 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__
-
-/* This code is used internally in the GNU libc.  */
-#ifdef __LIBC_INTERNAL_MATH_INLINES
-
-__MATH_INLINE double
-__NTH (__ieee754_sqrt (double x))
-{
-  double res;
-
-  __asm__ ( "sqdbr %0,%1" : "=f" (res) : "f" (x) );
-  return res;
-}
-
-__MATH_INLINE float
-__NTH (__ieee754_sqrtf (float x))
-{
-  float res;
-
-  __asm__ ( "sqebr %0,%1" : "=f" (res) : "f" (x) );
-  return res;
-}
-
-# if !defined __NO_LONG_DOUBLE_MATH
-__MATH_INLINE long double
-__NTH (sqrtl (long double __x))
-{
-  long double res;
-
-  __asm__ ( "sqxbr %0,%1" : "=f" (res) : "f" (__x) );
-  return res;
-}
-# endif /* !__NO_LONG_DOUBLE_MATH */
-
-#endif /* __LIBC_INTERNAL_MATH_INLINES */
-
-#endif /* __NO_MATH_INLINES */
diff --git a/sysdeps/sparc/fpu/bits/mathinline.h b/sysdeps/sparc/fpu/bits/mathinline.h
index 5940a1d0a00224d43715da715986ead6c48b4322..f6118c7ca50bb3630a81e6b415dea77aad8f5c68 100644
--- a/sysdeps/sparc/fpu/bits/mathinline.h
+++ b/sysdeps/sparc/fpu/bits/mathinline.h
@@ -33,86 +33,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)
-
-#  if !defined __NO_MATH_INLINES && !__GNUC_PREREQ (3, 2)
-
-__MATH_INLINE double
-__NTH (sqrt (double __x))
-{
-  register double __r;
-  __asm ("fsqrtd %1,%0" : "=f" (__r) : "f" (__x));
-  return __r;
-}
-
-__MATH_INLINE float
-__NTH (sqrtf (float __x))
-{
-  register float __r;
-  __asm ("fsqrts %1,%0" : "=f" (__r) : "f" (__x));
-  return __r;
-}
-
-#   if __WORDSIZE == 64
-__MATH_INLINE long double
-__NTH (sqrtl (long double __x))
-{
-  long double __r;
-  extern void _Qp_sqrt (long double *, const long double *);
-  _Qp_sqrt (&__r, &__x);
-  return __r;
-}
-#   elif !defined __NO_LONG_DOUBLE_MATH
-__MATH_INLINE long double
-sqrtl (long double __x) __THROW
-{
-  extern long double _Q_sqrt (const long double);
-  return _Q_sqrt (__x);
-}
-#   endif /* sparc64 */
-
-#  endif /* !__NO_MATH_INLINES && !GCC 3.2+ */
-
-/* This code is used internally in the GNU libc.  */
-#  ifdef __LIBC_INTERNAL_MATH_INLINES
-__MATH_INLINE double
-__ieee754_sqrt (double __x)
-{
-  register double __r;
-  __asm ("fsqrtd %1,%0" : "=f" (__r) : "f" (__x));
-  return __r;
-}
-
-__MATH_INLINE float
-__ieee754_sqrtf (float __x)
-{
-  register float __r;
-  __asm ("fsqrts %1,%0" : "=f" (__r) : "f" (__x));
-  return __r;
-}
-
-#   if __WORDSIZE == 64
-__MATH_INLINE long double
-__ieee754_sqrtl (long double __x)
-{
-  long double __r;
-  extern void _Qp_sqrt (long double *, const long double *);
-  _Qp_sqrt(&__r, &__x);
-  return __r;
-}
-#   elif !defined __NO_LONG_DOUBLE_MATH
-__MATH_INLINE long double
-__ieee754_sqrtl (long double __x)
-{
-  extern long double _Q_sqrt (const long double);
-  return _Q_sqrt (__x);
-}
-#   endif /* sparc64 */
-#  endif /* __LIBC_INTERNAL_MATH_INLINES */
-# endif /* gcc 2.8+ */
-
 # ifdef __USE_ISOC99
 
 #  ifndef __NO_MATH_INLINES
diff --git a/sysdeps/x86/fpu/bits/mathinline.h b/sysdeps/x86/fpu/bits/mathinline.h
index aed2a8febb7987789c08556d618afb2010e099ff..9b7a28f83e38ff5e40f20f6d0236020fb73961d0 100644
--- a/sysdeps/x86/fpu/bits/mathinline.h
+++ b/sysdeps/x86/fpu/bits/mathinline.h
@@ -819,7 +819,6 @@  __NTH (__finite (double __x))
 
 /* This code is used internally in the GNU libc.  */
 # ifdef __LIBC_INTERNAL_MATH_INLINES
-__inline_mathop (__ieee754_sqrt, "fsqrt")
 __inline_mathcode2_ (long double, __ieee754_atan2l, __y, __x,
 		     register long double __value;
 		     __asm __volatile__ ("fpatan\n\t"
diff --git a/sysdeps/x86_64/fpu/math_private.h b/sysdeps/x86_64/fpu/math_private.h
index 027a6a3a4d0a68948f19ebaa129bf442a9f4f3e6..13052893ef9541e100d6d68b85ff16a2910a28e3 100644
--- a/sysdeps/x86_64/fpu/math_private.h
+++ b/sysdeps/x86_64/fpu/math_private.h
@@ -48,38 +48,6 @@ 
 #include <sysdeps/i386/fpu/fenv_private.h>
 #include_next <math_private.h>
 
-extern __always_inline double
-__ieee754_sqrt (double d)
-{
-  double res;
-#if defined __AVX__ || defined SSE2AVX
-  asm ("vsqrtsd %1, %0, %0" : "=x" (res) : "xm" (d));
-#else
-  asm ("sqrtsd %1, %0" : "=x" (res) : "xm" (d));
-#endif
-  return res;
-}
-
-extern __always_inline float
-__ieee754_sqrtf (float d)
-{
-  float res;
-#if defined __AVX__ || defined SSE2AVX
-  asm ("vsqrtss %1, %0, %0" : "=x" (res) : "xm" (d));
-#else
-  asm ("sqrtss %1, %0" : "=x" (res) : "xm" (d));
-#endif
-  return res;
-}
-
-extern __always_inline long double
-__ieee754_sqrtl (long double d)
-{
-  long double res;
-  asm ("fsqrt" : "=t" (res) : "0" (d));
-  return res;
-}
-
 #ifdef __SSE4_1__
 extern __always_inline double
 __rint (double d)