unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v2 1/3] math: Remove fenvinline.h
@ 2020-03-09 18:32 Adhemerval Zanella
  2020-03-09 18:32 ` [PATCH 2/3] x86: Remove feraiseexcept optimization Adhemerval Zanella
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2020-03-09 18:32 UTC (permalink / raw
  To: libc-alpha

Changes from previous version:

  - Mention on commit message x86 also exports a similar optimization,
    but on a different header.

--

Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
patch removes the fenvinline.h on all architectures.  Currently
only powerpc implements some optimizations.  This kind of optimization
is better implemented by the compiler (which handles the architecture
ISA transparently).

Also, for the specific optimized powerpc implementation the code is
becoming convoluted and these micro-optimization are hardly wildly
used, even more being a possible hotspot in realword cases
(non-default rounding are used only on specific cases and exception
handling are done most likely only on errors path).  Only x86
implements similar optimization (on fenv.h) also indicates that
these should no be on libc.

The math/test-fenv already covers all math/test-fenvinline tests,
so it is safe to remove it.

Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
---
 bits/fenvinline.h                 |   8 -
 math/Makefile                     |   4 +-
 math/fenv.h                       |   4 -
 math/test-fenvinline.c            | 354 ------------------------------
 sysdeps/powerpc/bits/fenvinline.h | 108 ---------
 5 files changed, 2 insertions(+), 476 deletions(-)
 delete mode 100644 bits/fenvinline.h
 delete mode 100644 math/test-fenvinline.c
 delete mode 100644 sysdeps/powerpc/bits/fenvinline.h

diff --git a/bits/fenvinline.h b/bits/fenvinline.h
deleted file mode 100644
index 42f77b5618..0000000000
--- a/bits/fenvinline.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* This file provides inline versions of floating-pint environment
-   handling functions.  If there were any.  */
-
-#ifndef __NO_MATH_INLINES
-
-/* Here is where the code would go.  */
-
-#endif
diff --git a/math/Makefile b/math/Makefile
index 84a8b94c74..f916594a51 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -24,7 +24,7 @@ include ../Makeconfig
 # Installed header files.
 headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
-		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
+		   bits/fenv.h bits/mathdef.h tgmath.h \
 		   bits/math-vector.h finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
@@ -233,7 +233,7 @@ tests = test-matherr-3 test-fenv basic-test \
 	test-misc test-fpucw test-fpucw-ieee tst-definitions test-tgmath \
 	test-tgmath-ret bug-nextafter bug-nexttoward bug-tgmath1 \
 	test-tgmath-int test-tgmath2 test-powl tst-CMPLX tst-CMPLX2 test-snan \
-	test-fenv-tls test-fenv-preserve test-fenv-return test-fenvinline \
+	test-fenv-tls test-fenv-preserve test-fenv-return \
 	test-nearbyint-except test-fenv-clear \
 	test-nearbyint-except-2 test-signgam-uchar test-signgam-uchar-init \
 	test-signgam-uint test-signgam-uint-init test-signgam-ullong \
diff --git a/math/fenv.h b/math/fenv.h
index 6cad1d3575..e6b9578d6c 100644
--- a/math/fenv.h
+++ b/math/fenv.h
@@ -140,10 +140,6 @@ extern int fegetmode (femode_t *__modep) __THROW;
 extern int fesetmode (const femode_t *__modep) __THROW;
 #endif
 
-/* Include optimization.  */
-#ifdef __OPTIMIZE__
-# include <bits/fenvinline.h>
-#endif
 
 /* NaN support.  */
 
diff --git a/math/test-fenvinline.c b/math/test-fenvinline.c
deleted file mode 100644
index 0e5d361fff..0000000000
--- a/math/test-fenvinline.c
+++ /dev/null
@@ -1,354 +0,0 @@
-/* Test for fenv inline implementations.
-   Copyright (C) 2015-2020 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 _GNU_SOURCE
-# define _GNU_SOURCE
-#endif
-
-/* To make sure the fenv inline function are used.  */
-#undef __NO_MATH_INLINES
-
-#include <fenv.h>
-#include <stdio.h>
-#include <math-tests.h>
-
-/*
-  Since not all architectures might define all exceptions, we define
-  a private set and map accordingly.
-*/
-#define NO_EXC 0
-#define INEXACT_EXC 0x1
-#define DIVBYZERO_EXC 0x2
-#define UNDERFLOW_EXC 0x04
-#define OVERFLOW_EXC 0x08
-#define INVALID_EXC 0x10
-#define ALL_EXC \
-        (INEXACT_EXC | DIVBYZERO_EXC | UNDERFLOW_EXC | OVERFLOW_EXC \
-         | INVALID_EXC)
-static int count_errors;
-
-#if FE_ALL_EXCEPT
-static void
-test_single_exception_fp_int (int exception,
-			      int exc_flag,
-			      int fe_flag,
-			      const char *flag_name)
-{
-  if (exception & exc_flag)
-    {
-      if (fetestexcept (fe_flag))
-        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
-      else
-        {
-          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
-          ++count_errors;
-        }
-    }
-  else
-    {
-      if (fetestexcept (fe_flag))
-        {
-          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
-          ++count_errors;
-        }
-      else
-        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
-    }
-}
-/* Test whether a given exception was raised.  */
-static void
-test_single_exception_fp_double (int exception,
-				 int exc_flag,
-				 double fe_flag,
-				 const char *flag_name)
-{
-  if (exception & exc_flag)
-    {
-      if (fetestexcept (fe_flag))
-        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
-      else
-        {
-          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
-          ++count_errors;
-        }
-    }
-  else
-    {
-      if (fetestexcept (fe_flag))
-        {
-          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
-          ++count_errors;
-        }
-      else
-        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
-    }
-}
-#endif
-
-static void
-test_exceptions (const char *test_name, int exception)
-{
-  printf ("Test: %s\n", test_name);
-#ifdef FE_DIVBYZERO
-  test_single_exception_fp_double (exception, DIVBYZERO_EXC, FE_DIVBYZERO,
-				   "DIVBYZERO");
-#endif
-#ifdef FE_INVALID
-  test_single_exception_fp_double (exception, INVALID_EXC, FE_INVALID,
-				   "INVALID");
-#endif
-#ifdef FE_INEXACT
-  test_single_exception_fp_double (exception, INEXACT_EXC, FE_INEXACT,
-				   "INEXACT");
-#endif
-#ifdef FE_UNDERFLOW
-  test_single_exception_fp_double (exception, UNDERFLOW_EXC, FE_UNDERFLOW,
-				   "UNDERFLOW");
-#endif
-#ifdef FE_OVERFLOW
-  test_single_exception_fp_double (exception, OVERFLOW_EXC, FE_OVERFLOW,
-				   "OVERFLOW");
-#endif
-}
-
-static void
-test_exceptionflag (void)
-{
-  printf ("Test: fegetexceptionflag (FE_ALL_EXCEPT)\n");
-#if FE_ALL_EXCEPT
-  fexcept_t excepts;
-
-  feclearexcept (FE_ALL_EXCEPT);
-
-  feraiseexcept (FE_INVALID);
-  fegetexceptflag (&excepts, FE_ALL_EXCEPT);
-
-  feclearexcept (FE_ALL_EXCEPT);
-  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
-
-  fesetexceptflag (&excepts, FE_ALL_EXCEPT);
-
-  test_single_exception_fp_int (INVALID_EXC, INVALID_EXC, FE_INVALID,
-				"INVALID (int)");
-  test_single_exception_fp_int (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
-				"OVERFLOW (int)");
-  test_single_exception_fp_int (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
-				"INEXACT (int)");
-
-  /* Same test, but using double as argument  */
-  feclearexcept (FE_ALL_EXCEPT);
-
-  feraiseexcept (FE_INVALID);
-  fegetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
-
-  feclearexcept (FE_ALL_EXCEPT);
-  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
-
-  fesetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
-
-  test_single_exception_fp_double (INVALID_EXC, INVALID_EXC, FE_INVALID,
-				   "INVALID (double)");
-  test_single_exception_fp_double (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
-				   "OVERFLOW (double)");
-  test_single_exception_fp_double (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
-				   "INEXACT (double)");
-#endif
-}
-
-static void
-test_fesetround (void)
-{
-#if defined FE_TONEAREST && defined FE_TOWARDZERO
-  int res1;
-  int res2;
-
-  printf ("Tests for fesetround\n");
-
-  /* The fesetround should not itself cause the test to fail, however it
-     should either succeed for both 'int' and 'double' argument, or fail
-     for both.  */
-  res1 = fesetround ((int) FE_TOWARDZERO);
-  res2 = fesetround ((double) FE_TOWARDZERO);
-  if (res1 != res2)
-    {
-      printf ("fesetround (FE_TOWARDZERO) failed: %d, %d\n", res1, res2);
-      ++count_errors;
-    }
-
-  res1 = fesetround ((int) FE_TONEAREST);
-  res2 = fesetround ((double) FE_TONEAREST);
-  if (res1 != res2)
-    {
-      printf ("fesetround (FE_TONEAREST) failed: %d, %d\n", res1, res2);
-      ++count_errors;
-    }
-#endif
-}
-
-#if FE_ALL_EXCEPT
-/* Tests for feenableexcept/fedisableexcept.  */
-static void
-feenable_test (const char *flag_name, fexcept_t fe_exc)
-{
-  int fe_exci = fe_exc;
-  double fe_excd = fe_exc;
-  int excepts;
-
-  /* First disable all exceptions.  */
-  if (fedisableexcept (FE_ALL_EXCEPT) == -1)
-    {
-      printf ("Test: fedisableexcept (FE_ALL_EXCEPT) failed\n");
-      ++count_errors;
-      /* If this fails, the other tests don't make sense.  */
-      return;
-    }
-
-  /* Test for inline macros using integer argument.  */
-  excepts = feenableexcept (fe_exci);
-  if (!EXCEPTION_ENABLE_SUPPORTED (fe_exci) && excepts == -1)
-    {
-      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
-      return;
-    }
-  if (excepts == -1)
-    {
-      printf ("Test: feenableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != 0)
-    {
-      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
-              flag_name, excepts);
-      ++count_errors;
-    }
-
-  /* And now disable the exception again.  */
-  excepts = fedisableexcept (fe_exc);
-  if (excepts == -1)
-    {
-      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != fe_exc)
-    {
-      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
-              flag_name, (unsigned int)fe_exc, excepts);
-      ++count_errors;
-    }
-
-  /* Test for inline macros using double argument.  */
-  excepts = feenableexcept (fe_excd);
-  if (!EXCEPTION_ENABLE_SUPPORTED (fe_excd) && excepts == -1)
-    {
-      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
-      return;
-    }
-  if (excepts == -1)
-    {
-      printf ("Test: feenableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != 0)
-    {
-      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
-              flag_name, excepts);
-      ++count_errors;
-    }
-
-  /* And now disable the exception again.  */
-  excepts = fedisableexcept (fe_exc);
-  if (excepts == -1)
-    {
-      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != fe_exc)
-    {
-      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
-              flag_name, (unsigned int)fe_exc, excepts);
-      ++count_errors;
-    }
-}
-#endif
-
-static void
-test_feenabledisable (void)
-{
-  printf ("Tests for feenableexcepts/fedisableexcept\n");
-
-  /* We might have some exceptions still set.  */
-  feclearexcept (FE_ALL_EXCEPT);
-
-#ifdef FE_DIVBYZERO
-  feenable_test ("FE_DIVBYZERO", FE_DIVBYZERO);
-#endif
-#ifdef FE_INVALID
-  feenable_test ("FE_INVALID", FE_INVALID);
-#endif
-#ifdef FE_INEXACT
-  feenable_test ("FE_INEXACT", FE_INEXACT);
-#endif
-#ifdef FE_UNDERFLOW
-  feenable_test ("FE_UNDERFLOW", FE_UNDERFLOW);
-#endif
-#ifdef FE_OVERFLOW
-  feenable_test ("FE_OVERFLOW", FE_OVERFLOW);
-#endif
-  fesetenv (FE_DFL_ENV);
-}
-
-static int
-do_test (void)
-{
-  /* clear all exceptions and test if all are cleared  */
-  feclearexcept (FE_ALL_EXCEPT);
-  test_exceptions ("feclearexcept (FE_ALL_EXCEPT) clears all exceptions",
-                   NO_EXC);
-
-  /* raise all exceptions and test if all are raised  */
-  feraiseexcept (FE_ALL_EXCEPT);
-  if (EXCEPTION_TESTS (float))
-    test_exceptions ("feraiseexcept (FE_ALL_EXCEPT) raises all exceptions",
-		     ALL_EXC);
-
-  /* Same test, but using double as argument  */
-  feclearexcept ((double)FE_ALL_EXCEPT);
-  test_exceptions ("feclearexcept ((double)FE_ALL_EXCEPT) clears all exceptions",
-                   NO_EXC);
-
-  feraiseexcept ((double)FE_ALL_EXCEPT);
-  if (EXCEPTION_TESTS (float))
-    test_exceptions ("feraiseexcept ((double)FE_ALL_EXCEPT) raises all exceptions",
-		     ALL_EXC);
-
-  if (EXCEPTION_TESTS (float))
-    test_exceptionflag ();
-
-  test_fesetround ();
-
-  test_feenabledisable ();
-
-  return count_errors;
-}
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
deleted file mode 100644
index f2d095a72f..0000000000
--- a/sysdeps/powerpc/bits/fenvinline.h
+++ /dev/null
@@ -1,108 +0,0 @@
-/* Inline floating-point environment handling functions for powerpc.
-   Copyright (C) 1995-2020 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/>.  */
-
-#if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
-
-/* Inline definitions for fegetround.  */
-# define __fegetround_ISA300()						\
-  (__extension__  ({							\
-    union { double __d; unsigned long long __ll; } __u;			\
-    __asm__ __volatile__ (						\
-      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
-      : "=f" (__u.__d));						\
-    __u.__ll & 0x0000000000000003LL;					\
-  }))
-
-# define __fegetround_ISA2()						\
-  (__extension__  ({							\
-     int __fegetround_result;						\
-     __asm__ __volatile__ ("mcrfs 7,7 ; mfcr %0"			\
-			   : "=r"(__fegetround_result) : : "cr7");	\
-     __fegetround_result & 3;						\
-  }))
-
-# ifdef _ARCH_PWR9
-#  define __fegetround() __fegetround_ISA300()
-# elif defined __BUILTIN_CPU_SUPPORTS__
-#  define __fegetround()						\
-  (__glibc_likely (__builtin_cpu_supports ("arch_3_00"))		\
-   ? __fegetround_ISA300()						\
-   : __fegetround_ISA2()						\
-  )
-# else
-#  define __fegetround() __fegetround_ISA2()
-# endif
-
-# define fegetround() __fegetround ()
-
-# ifndef __NO_MATH_INLINES
-
-/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
-#  if !__GNUC_PREREQ(9, 0)
-/* The weird 'i#*X' constraints on the following suppress a gcc
-   warning when __excepts is not a constant.  Otherwise, they mean the
-   same as just plain 'i'.  This warning only happens in old GCC
-   versions (gcc 3 or less).  Otherwise plain 'i' works fine.  */
-#   define __MTFSB0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b))
-#   define __MTFSB1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b))
-#  else
-#   define __MTFSB0(__b) __builtin_mtfsb0 (__b)
-#   define __MTFSB1(__b) __builtin_mtfsb1 (__b)
-#  endif
-
-#  if __GNUC_PREREQ(3, 4)
-
-#include <sys/param.h>
-
-/* Inline definition for feraiseexcept.  */
-#   define feraiseexcept(__excepts) \
-  (__extension__  ({ 							      \
-    int __e = __excepts;						      \
-    int __ret = 0;							      \
-    if (__builtin_constant_p (__e)					      \
-        && __builtin_popcount (__e) == 1				      \
-        && __e != FE_INVALID)						      \
-      {									      \
-	__MTFSB1 ((__builtin_clz (__e)));				      \
-      }									      \
-    else								      \
-      __ret = feraiseexcept (__e);					      \
-    __ret;								      \
-  }))
-
-/* Inline definition for feclearexcept.  */
-#   define feclearexcept(__excepts) \
-  (__extension__  ({ 							      \
-    int __e = __excepts;						      \
-    int __ret = 0;							      \
-    if (__builtin_constant_p (__e)					      \
-        && __builtin_popcount (__e) == 1				      \
-        && __e != FE_INVALID)						      \
-      {									      \
-	__MTFSB0 ((__builtin_clz (__e)));				      \
-      }									      \
-    else								      \
-      __ret = feclearexcept (__e);					      \
-    __ret;								      \
-  }))
-
-#  endif /* __GNUC_PREREQ(3, 4).  */
-
-# endif /* !__NO_MATH_INLINES.  */
-
-#endif /* __GNUC__ && !_SOFT_FLOAT && !__NO_FPRS__ */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] x86: Remove feraiseexcept optimization
  2020-03-09 18:32 [PATCH v2 1/3] math: Remove fenvinline.h Adhemerval Zanella
@ 2020-03-09 18:32 ` Adhemerval Zanella
  2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
  2020-03-09 18:32 ` [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h Adhemerval Zanella
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2020-03-09 18:32 UTC (permalink / raw
  To: libc-alpha

Similar to fenvinline.h removal, this kind of optimization is better
implemented by the compiler.  Also newer code avoid setting exceptions
directly (for instance the code to make new logf, log2f and powf
implementatation to now support SVID compat).

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/x86/fpu/bits/fenv.h         | 54 ---------------------------
 sysdeps/x86/fpu/include/bits/fenv.h | 57 -----------------------------
 2 files changed, 111 deletions(-)
 delete mode 100644 sysdeps/x86/fpu/include/bits/fenv.h

diff --git a/sysdeps/x86/fpu/bits/fenv.h b/sysdeps/x86/fpu/bits/fenv.h
index eb78eb4b2d..6cfa5678eb 100644
--- a/sysdeps/x86/fpu/bits/fenv.h
+++ b/sysdeps/x86/fpu/bits/fenv.h
@@ -114,57 +114,3 @@ femode_t;
 /* Default floating-point control modes.  */
 # define FE_DFL_MODE	((const femode_t *) -1L)
 #endif
-
-
-#ifdef __USE_EXTERN_INLINES
-__BEGIN_DECLS
-
-/* Optimized versions.  */
-#ifndef _LIBC
-extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
-#endif
-__extern_always_inline void
-__NTH (__feraiseexcept_invalid_divbyzero (int __excepts))
-{
-  if ((FE_INVALID & __excepts) != 0)
-    {
-      /* One example of an invalid operation is 0.0 / 0.0.  */
-      float __f = 0.0;
-
-# ifdef __SSE_MATH__
-      __asm__ __volatile__ ("divss %0, %0 " : : "x" (__f));
-# else
-      __asm__ __volatile__ ("fdiv %%st, %%st(0); fwait"
-			    : "=t" (__f) : "0" (__f));
-# endif
-      (void) &__f;
-    }
-  if ((FE_DIVBYZERO & __excepts) != 0)
-    {
-      float __f = 1.0;
-      float __g = 0.0;
-
-# ifdef __SSE_MATH__
-      __asm__ __volatile__ ("divss %1, %0" : : "x" (__f), "x" (__g));
-# else
-      __asm__ __volatile__ ("fdivp %%st, %%st(1); fwait"
-			    : "=t" (__f) : "0" (__f), "u" (__g) : "st(1)");
-# endif
-      (void) &__f;
-    }
-}
-__extern_inline int
-__NTH (feraiseexcept (int __excepts))
-{
-  if (__builtin_constant_p (__excepts)
-      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
-    {
-      __feraiseexcept_invalid_divbyzero (__excepts);
-      return 0;
-    }
-
-  return __feraiseexcept_renamed (__excepts);
-}
-
-__END_DECLS
-#endif
diff --git a/sysdeps/x86/fpu/include/bits/fenv.h b/sysdeps/x86/fpu/include/bits/fenv.h
deleted file mode 100644
index dd3f61e9f3..0000000000
--- a/sysdeps/x86/fpu/include/bits/fenv.h
+++ /dev/null
@@ -1,57 +0,0 @@
-/* Wrapper for x86 bits/fenv.h for use when building glibc.
-   Copyright (C) 1997-2020 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 _BITS_FENV_H
-
-#if defined _LIBC && defined __USE_EXTERN_INLINES
-# if defined SHARED && !defined NO_HIDDEN && IS_IN (libm)
-extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), __GI_feraiseexcept);
-# else
-extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
-# endif
-#endif
-
-#include_next <bits/fenv.h>
-
-# ifndef _ISOMAC
-
-/* Ensure __feraiseexcept calls in glibc are optimized the same as
-   feraiseexcept calls.  */
-
-#ifdef __USE_EXTERN_INLINES
-__BEGIN_DECLS
-
-extern int __REDIRECT_NTH (____feraiseexcept_renamed, (int), __feraiseexcept);
-__extern_inline int
-__NTH (__feraiseexcept (int __excepts))
-{
-  if (__builtin_constant_p (__excepts)
-      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
-    {
-      __feraiseexcept_invalid_divbyzero (__excepts);
-      return 0;
-    }
-
-  return ____feraiseexcept_renamed (__excepts);
-}
-
-__END_DECLS
-#endif
-
-# endif /* _ISOMAC */
-#endif /* bits/fenv.h */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h
  2020-03-09 18:32 [PATCH v2 1/3] math: Remove fenvinline.h Adhemerval Zanella
  2020-03-09 18:32 ` [PATCH 2/3] x86: Remove feraiseexcept optimization Adhemerval Zanella
@ 2020-03-09 18:32 ` Adhemerval Zanella
  2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
  2020-03-09 20:19 ` [PATCH v2 1/3] math: Remove fenvinline.h Paul E Murphy
  2020-03-10 11:29 ` Florian Weimer
  3 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella @ 2020-03-09 18:32 UTC (permalink / raw
  To: libc-alpha

These should not be exported on installed headers.

Checked on sparc64-linux-gnu and sparcv9-linux-gnu.
---
 sysdeps/sparc/fpu/bits/fenv.h    | 9 ---------
 sysdeps/sparc/fpu/fclrexcpt.c    | 1 +
 sysdeps/sparc/fpu/fedisblxcpt.c  | 1 +
 sysdeps/sparc/fpu/feenablxcpt.c  | 1 +
 sysdeps/sparc/fpu/fegetenv.c     | 1 +
 sysdeps/sparc/fpu/fegetexcept.c  | 1 +
 sysdeps/sparc/fpu/fegetmode.c    | 1 +
 sysdeps/sparc/fpu/fegetround.c   | 1 +
 sysdeps/sparc/fpu/feholdexcpt.c  | 1 +
 sysdeps/sparc/fpu/fenv_private.h | 9 +++++++++
 sysdeps/sparc/fpu/fesetenv.c     | 1 +
 sysdeps/sparc/fpu/fesetexcept.c  | 1 +
 sysdeps/sparc/fpu/fesetmode.c    | 1 +
 sysdeps/sparc/fpu/fesetround.c   | 1 +
 sysdeps/sparc/fpu/feupdateenv.c  | 1 +
 sysdeps/sparc/fpu/fgetexcptflg.c | 1 +
 sysdeps/sparc/fpu/fsetexcptflg.c | 1 +
 sysdeps/sparc/fpu/ftestexcept.c  | 1 +
 18 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/sysdeps/sparc/fpu/bits/fenv.h b/sysdeps/sparc/fpu/bits/fenv.h
index 4935208a41..d34fcac4c5 100644
--- a/sysdeps/sparc/fpu/bits/fenv.h
+++ b/sysdeps/sparc/fpu/bits/fenv.h
@@ -83,15 +83,6 @@ typedef unsigned long int fenv_t;
 # define FE_NOMASK_ENV	((const fenv_t *) -2)
 #endif
 
-/* For internal use only: access the fp state register.  */
-#if __WORDSIZE == 64
-# define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
-# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
-#else
-# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
-# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
-#endif
-
 #if __GLIBC_USE (IEC_60559_BFP_EXT_C2X)
 /* Type representing floating-point control modes.  */
 typedef unsigned long int femode_t;
diff --git a/sysdeps/sparc/fpu/fclrexcpt.c b/sysdeps/sparc/fpu/fclrexcpt.c
index b11734f057..5af20d1f7a 100644
--- a/sysdeps/sparc/fpu/fclrexcpt.c
+++ b/sysdeps/sparc/fpu/fclrexcpt.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <shlib-compat.h>
 
 int
diff --git a/sysdeps/sparc/fpu/fedisblxcpt.c b/sysdeps/sparc/fpu/fedisblxcpt.c
index 86688ab533..9b832a82ce 100644
--- a/sysdeps/sparc/fpu/fedisblxcpt.c
+++ b/sysdeps/sparc/fpu/fedisblxcpt.c
@@ -18,6 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 fedisableexcept (int excepts)
diff --git a/sysdeps/sparc/fpu/feenablxcpt.c b/sysdeps/sparc/fpu/feenablxcpt.c
index 647093cebc..06ec14cee5 100644
--- a/sysdeps/sparc/fpu/feenablxcpt.c
+++ b/sysdeps/sparc/fpu/feenablxcpt.c
@@ -18,6 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 feenableexcept (int excepts)
diff --git a/sysdeps/sparc/fpu/fegetenv.c b/sysdeps/sparc/fpu/fegetenv.c
index edde6ae5b2..00c0bc72b5 100644
--- a/sysdeps/sparc/fpu/fegetenv.c
+++ b/sysdeps/sparc/fpu/fegetenv.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <shlib-compat.h>
 
 int
diff --git a/sysdeps/sparc/fpu/fegetexcept.c b/sysdeps/sparc/fpu/fegetexcept.c
index f549a90190..4d9746dd57 100644
--- a/sysdeps/sparc/fpu/fegetexcept.c
+++ b/sysdeps/sparc/fpu/fegetexcept.c
@@ -18,6 +18,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 fegetexcept (void)
diff --git a/sysdeps/sparc/fpu/fegetmode.c b/sysdeps/sparc/fpu/fegetmode.c
index 18c932d520..aa160bd19a 100644
--- a/sysdeps/sparc/fpu/fegetmode.c
+++ b/sysdeps/sparc/fpu/fegetmode.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 fegetmode (femode_t *modep)
diff --git a/sysdeps/sparc/fpu/fegetround.c b/sysdeps/sparc/fpu/fegetround.c
index 1eae341fc4..6ca7d5c0dc 100644
--- a/sysdeps/sparc/fpu/fegetround.c
+++ b/sysdeps/sparc/fpu/fegetround.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 __fegetround (void)
diff --git a/sysdeps/sparc/fpu/feholdexcpt.c b/sysdeps/sparc/fpu/feholdexcpt.c
index 7a1a3e33ed..bb612402f0 100644
--- a/sysdeps/sparc/fpu/feholdexcpt.c
+++ b/sysdeps/sparc/fpu/feholdexcpt.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 __feholdexcept (fenv_t *envp)
diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
index dbd1001ccb..da7c7fe332 100644
--- a/sysdeps/sparc/fpu/fenv_private.h
+++ b/sysdeps/sparc/fpu/fenv_private.h
@@ -3,6 +3,15 @@
 
 #include <fenv.h>
 
+/* For internal use only: access the fp state register.  */
+#if __WORDSIZE == 64
+# define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
+# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
+#else
+# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
+# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
+#endif
+
 static __always_inline void
 libc_feholdexcept (fenv_t *e)
 {
diff --git a/sysdeps/sparc/fpu/fesetenv.c b/sysdeps/sparc/fpu/fesetenv.c
index 82c03c6760..d536abd344 100644
--- a/sysdeps/sparc/fpu/fesetenv.c
+++ b/sysdeps/sparc/fpu/fesetenv.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <shlib-compat.h>
 
 int
diff --git a/sysdeps/sparc/fpu/fesetexcept.c b/sysdeps/sparc/fpu/fesetexcept.c
index 6740ece5b4..fbc21c0477 100644
--- a/sysdeps/sparc/fpu/fesetexcept.c
+++ b/sysdeps/sparc/fpu/fesetexcept.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 fesetexcept (int excepts)
diff --git a/sysdeps/sparc/fpu/fesetmode.c b/sysdeps/sparc/fpu/fesetmode.c
index 6fe5d337ad..24148e0fd3 100644
--- a/sysdeps/sparc/fpu/fesetmode.c
+++ b/sysdeps/sparc/fpu/fesetmode.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <fpu_control.h>
 
 #define FPU_CONTROL_BITS 0xcfc00000UL
diff --git a/sysdeps/sparc/fpu/fesetround.c b/sysdeps/sparc/fpu/fesetround.c
index 9a944322d7..b259474d2c 100644
--- a/sysdeps/sparc/fpu/fesetround.c
+++ b/sysdeps/sparc/fpu/fesetround.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 __fesetround (int round)
diff --git a/sysdeps/sparc/fpu/feupdateenv.c b/sysdeps/sparc/fpu/feupdateenv.c
index 7e2399bfa2..7721f822ea 100644
--- a/sysdeps/sparc/fpu/feupdateenv.c
+++ b/sysdeps/sparc/fpu/feupdateenv.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <shlib-compat.h>
 
 int
diff --git a/sysdeps/sparc/fpu/fgetexcptflg.c b/sysdeps/sparc/fpu/fgetexcptflg.c
index f95d9bbf1b..ab8fa1bb76 100644
--- a/sysdeps/sparc/fpu/fgetexcptflg.c
+++ b/sysdeps/sparc/fpu/fgetexcptflg.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <shlib-compat.h>
 
 int
diff --git a/sysdeps/sparc/fpu/fsetexcptflg.c b/sysdeps/sparc/fpu/fsetexcptflg.c
index 077dfc9953..34eb789a94 100644
--- a/sysdeps/sparc/fpu/fsetexcptflg.c
+++ b/sysdeps/sparc/fpu/fsetexcptflg.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 #include <math.h>
 #include <shlib-compat.h>
 
diff --git a/sysdeps/sparc/fpu/ftestexcept.c b/sysdeps/sparc/fpu/ftestexcept.c
index a8c8e06ef6..44367ab4fd 100644
--- a/sysdeps/sparc/fpu/ftestexcept.c
+++ b/sysdeps/sparc/fpu/ftestexcept.c
@@ -17,6 +17,7 @@
    <https://www.gnu.org/licenses/>.  */
 
 #include <fenv.h>
+#include <fenv_private.h>
 
 int
 fetestexcept (int excepts)
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-09 18:32 [PATCH v2 1/3] math: Remove fenvinline.h Adhemerval Zanella
  2020-03-09 18:32 ` [PATCH 2/3] x86: Remove feraiseexcept optimization Adhemerval Zanella
  2020-03-09 18:32 ` [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h Adhemerval Zanella
@ 2020-03-09 20:19 ` Paul E Murphy
  2020-03-10 16:51   ` Adhemerval Zanella
  2020-03-10 11:29 ` Florian Weimer
  3 siblings, 1 reply; 14+ messages in thread
From: Paul E Murphy @ 2020-03-09 20:19 UTC (permalink / raw
  To: Adhemerval Zanella, libc-alpha



On 3/9/20 1:32 PM, Adhemerval Zanella wrote:
> Changes from previous version:
> 
>    - Mention on commit message x86 also exports a similar optimization,
>      but on a different header.
> 
> --
> 
> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
> patch removes the fenvinline.h on all architectures.  Currently
> only powerpc implements some optimizations.  This kind of optimization
> is better implemented by the compiler (which handles the architecture
> ISA transparently).
> 
> Also, for the specific optimized powerpc implementation the code is
> becoming convoluted and these micro-optimization are hardly wildly
> used, even more being a possible hotspot in realword cases
> (non-default rounding are used only on specific cases and exception
> handling are done most likely only on errors path).  Only x86
> implements similar optimization (on fenv.h) also indicates that
> these should no be on libc.
> 
> The math/test-fenv already covers all math/test-fenvinline tests,
> so it is safe to remove it.
> 
> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
> ---
>   bits/fenvinline.h                 |   8 -
>   math/Makefile                     |   4 +-
>   math/fenv.h                       |   4 -
>   math/test-fenvinline.c            | 354 ------------------------------
>   sysdeps/powerpc/bits/fenvinline.h | 108 ---------
>   5 files changed, 2 insertions(+), 476 deletions(-)
>   delete mode 100644 bits/fenvinline.h
>   delete mode 100644 math/test-fenvinline.c
>   delete mode 100644 sysdeps/powerpc/bits/fenvinline.h

Does sysdeps/powerpc/fpu/fegetround.c also need updated with this patch 
too?  I think it using the removed __fegetround macro.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-09 18:32 [PATCH v2 1/3] math: Remove fenvinline.h Adhemerval Zanella
                   ` (2 preceding siblings ...)
  2020-03-09 20:19 ` [PATCH v2 1/3] math: Remove fenvinline.h Paul E Murphy
@ 2020-03-10 11:29 ` Florian Weimer
  2020-03-10 16:58   ` Adhemerval Zanella via Libc-alpha
  2020-03-10 22:54   ` Joseph Myers
  3 siblings, 2 replies; 14+ messages in thread
From: Florian Weimer @ 2020-03-10 11:29 UTC (permalink / raw
  To: Adhemerval Zanella; +Cc: libc-alpha

* Adhemerval Zanella:

> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
> patch removes the fenvinline.h on all architectures.  Currently
> only powerpc implements some optimizations.  This kind of optimization
> is better implemented by the compiler (which handles the architecture
> ISA transparently).
>
> Also, for the specific optimized powerpc implementation the code is
> becoming convoluted and these micro-optimization are hardly wildly
> used, even more being a possible hotspot in realword cases
> (non-default rounding are used only on specific cases and exception
> handling are done most likely only on errors path).  Only x86
> implements similar optimization (on fenv.h) also indicates that
> these should no be on libc.

Wasn't the idea to make this change only after the GCC optimizations
have been implemented?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-09 20:19 ` [PATCH v2 1/3] math: Remove fenvinline.h Paul E Murphy
@ 2020-03-10 16:51   ` Adhemerval Zanella
  2020-03-27 19:33     ` Adhemerval Zanella via Libc-alpha
  2020-03-27 21:43     ` Paul E Murphy via Libc-alpha
  0 siblings, 2 replies; 14+ messages in thread
From: Adhemerval Zanella @ 2020-03-10 16:51 UTC (permalink / raw
  To: Paul E Murphy, libc-alpha



On 09/03/2020 17:19, Paul E Murphy wrote:
> 
> 
> On 3/9/20 1:32 PM, Adhemerval Zanella wrote:
>> Changes from previous version:
>>
>>    - Mention on commit message x86 also exports a similar optimization,
>>      but on a different header.
>>
>> -- 
>>
>> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
>> patch removes the fenvinline.h on all architectures.  Currently
>> only powerpc implements some optimizations.  This kind of optimization
>> is better implemented by the compiler (which handles the architecture
>> ISA transparently).
>>
>> Also, for the specific optimized powerpc implementation the code is
>> becoming convoluted and these micro-optimization are hardly wildly
>> used, even more being a possible hotspot in realword cases
>> (non-default rounding are used only on specific cases and exception
>> handling are done most likely only on errors path).  Only x86
>> implements similar optimization (on fenv.h) also indicates that
>> these should no be on libc.
>>
>> The math/test-fenv already covers all math/test-fenvinline tests,
>> so it is safe to remove it.
>>
>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>> ---
>>   bits/fenvinline.h                 |   8 -
>>   math/Makefile                     |   4 +-
>>   math/fenv.h                       |   4 -
>>   math/test-fenvinline.c            | 354 ------------------------------
>>   sysdeps/powerpc/bits/fenvinline.h | 108 ---------
>>   5 files changed, 2 insertions(+), 476 deletions(-)
>>   delete mode 100644 bits/fenvinline.h
>>   delete mode 100644 math/test-fenvinline.c
>>   delete mode 100644 sysdeps/powerpc/bits/fenvinline.h

Indeed, I misread the failures on powerpc64le-linux-gnu.  Below it is
an updated patch with the fegetround optimization moved to an internal
header.

--

diff --git a/bits/fenvinline.h b/bits/fenvinline.h
deleted file mode 100644
index 42f77b5618..0000000000
--- a/bits/fenvinline.h
+++ /dev/null
@@ -1,8 +0,0 @@
-/* This file provides inline versions of floating-pint environment
-   handling functions.  If there were any.  */
-
-#ifndef __NO_MATH_INLINES
-
-/* Here is where the code would go.  */
-
-#endif
diff --git a/math/Makefile b/math/Makefile
index 84a8b94c74..f916594a51 100644
--- a/math/Makefile
+++ b/math/Makefile
@@ -24,7 +24,7 @@ include ../Makeconfig
 # Installed header files.
 headers		:= math.h bits/mathcalls.h bits/mathinline.h \
 		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
-		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
+		   bits/fenv.h bits/mathdef.h tgmath.h \
 		   bits/math-vector.h finclude/math-vector-fortran.h \
 		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
 		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
@@ -233,7 +233,7 @@ tests = test-matherr-3 test-fenv basic-test \
 	test-misc test-fpucw test-fpucw-ieee tst-definitions test-tgmath \
 	test-tgmath-ret bug-nextafter bug-nexttoward bug-tgmath1 \
 	test-tgmath-int test-tgmath2 test-powl tst-CMPLX tst-CMPLX2 test-snan \
-	test-fenv-tls test-fenv-preserve test-fenv-return test-fenvinline \
+	test-fenv-tls test-fenv-preserve test-fenv-return \
 	test-nearbyint-except test-fenv-clear \
 	test-nearbyint-except-2 test-signgam-uchar test-signgam-uchar-init \
 	test-signgam-uint test-signgam-uint-init test-signgam-ullong \
diff --git a/math/fenv.h b/math/fenv.h
index 6cad1d3575..e6b9578d6c 100644
--- a/math/fenv.h
+++ b/math/fenv.h
@@ -140,10 +140,6 @@ extern int fegetmode (femode_t *__modep) __THROW;
 extern int fesetmode (const femode_t *__modep) __THROW;
 #endif
 
-/* Include optimization.  */
-#ifdef __OPTIMIZE__
-# include <bits/fenvinline.h>
-#endif
 
 /* NaN support.  */
 
diff --git a/math/test-fenvinline.c b/math/test-fenvinline.c
deleted file mode 100644
index 0e5d361fff..0000000000
--- a/math/test-fenvinline.c
+++ /dev/null
@@ -1,354 +0,0 @@
-/* Test for fenv inline implementations.
-   Copyright (C) 2015-2020 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 _GNU_SOURCE
-# define _GNU_SOURCE
-#endif
-
-/* To make sure the fenv inline function are used.  */
-#undef __NO_MATH_INLINES
-
-#include <fenv.h>
-#include <stdio.h>
-#include <math-tests.h>
-
-/*
-  Since not all architectures might define all exceptions, we define
-  a private set and map accordingly.
-*/
-#define NO_EXC 0
-#define INEXACT_EXC 0x1
-#define DIVBYZERO_EXC 0x2
-#define UNDERFLOW_EXC 0x04
-#define OVERFLOW_EXC 0x08
-#define INVALID_EXC 0x10
-#define ALL_EXC \
-        (INEXACT_EXC | DIVBYZERO_EXC | UNDERFLOW_EXC | OVERFLOW_EXC \
-         | INVALID_EXC)
-static int count_errors;
-
-#if FE_ALL_EXCEPT
-static void
-test_single_exception_fp_int (int exception,
-			      int exc_flag,
-			      int fe_flag,
-			      const char *flag_name)
-{
-  if (exception & exc_flag)
-    {
-      if (fetestexcept (fe_flag))
-        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
-      else
-        {
-          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
-          ++count_errors;
-        }
-    }
-  else
-    {
-      if (fetestexcept (fe_flag))
-        {
-          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
-          ++count_errors;
-        }
-      else
-        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
-    }
-}
-/* Test whether a given exception was raised.  */
-static void
-test_single_exception_fp_double (int exception,
-				 int exc_flag,
-				 double fe_flag,
-				 const char *flag_name)
-{
-  if (exception & exc_flag)
-    {
-      if (fetestexcept (fe_flag))
-        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
-      else
-        {
-          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
-          ++count_errors;
-        }
-    }
-  else
-    {
-      if (fetestexcept (fe_flag))
-        {
-          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
-          ++count_errors;
-        }
-      else
-        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
-    }
-}
-#endif
-
-static void
-test_exceptions (const char *test_name, int exception)
-{
-  printf ("Test: %s\n", test_name);
-#ifdef FE_DIVBYZERO
-  test_single_exception_fp_double (exception, DIVBYZERO_EXC, FE_DIVBYZERO,
-				   "DIVBYZERO");
-#endif
-#ifdef FE_INVALID
-  test_single_exception_fp_double (exception, INVALID_EXC, FE_INVALID,
-				   "INVALID");
-#endif
-#ifdef FE_INEXACT
-  test_single_exception_fp_double (exception, INEXACT_EXC, FE_INEXACT,
-				   "INEXACT");
-#endif
-#ifdef FE_UNDERFLOW
-  test_single_exception_fp_double (exception, UNDERFLOW_EXC, FE_UNDERFLOW,
-				   "UNDERFLOW");
-#endif
-#ifdef FE_OVERFLOW
-  test_single_exception_fp_double (exception, OVERFLOW_EXC, FE_OVERFLOW,
-				   "OVERFLOW");
-#endif
-}
-
-static void
-test_exceptionflag (void)
-{
-  printf ("Test: fegetexceptionflag (FE_ALL_EXCEPT)\n");
-#if FE_ALL_EXCEPT
-  fexcept_t excepts;
-
-  feclearexcept (FE_ALL_EXCEPT);
-
-  feraiseexcept (FE_INVALID);
-  fegetexceptflag (&excepts, FE_ALL_EXCEPT);
-
-  feclearexcept (FE_ALL_EXCEPT);
-  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
-
-  fesetexceptflag (&excepts, FE_ALL_EXCEPT);
-
-  test_single_exception_fp_int (INVALID_EXC, INVALID_EXC, FE_INVALID,
-				"INVALID (int)");
-  test_single_exception_fp_int (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
-				"OVERFLOW (int)");
-  test_single_exception_fp_int (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
-				"INEXACT (int)");
-
-  /* Same test, but using double as argument  */
-  feclearexcept (FE_ALL_EXCEPT);
-
-  feraiseexcept (FE_INVALID);
-  fegetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
-
-  feclearexcept (FE_ALL_EXCEPT);
-  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
-
-  fesetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
-
-  test_single_exception_fp_double (INVALID_EXC, INVALID_EXC, FE_INVALID,
-				   "INVALID (double)");
-  test_single_exception_fp_double (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
-				   "OVERFLOW (double)");
-  test_single_exception_fp_double (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
-				   "INEXACT (double)");
-#endif
-}
-
-static void
-test_fesetround (void)
-{
-#if defined FE_TONEAREST && defined FE_TOWARDZERO
-  int res1;
-  int res2;
-
-  printf ("Tests for fesetround\n");
-
-  /* The fesetround should not itself cause the test to fail, however it
-     should either succeed for both 'int' and 'double' argument, or fail
-     for both.  */
-  res1 = fesetround ((int) FE_TOWARDZERO);
-  res2 = fesetround ((double) FE_TOWARDZERO);
-  if (res1 != res2)
-    {
-      printf ("fesetround (FE_TOWARDZERO) failed: %d, %d\n", res1, res2);
-      ++count_errors;
-    }
-
-  res1 = fesetround ((int) FE_TONEAREST);
-  res2 = fesetround ((double) FE_TONEAREST);
-  if (res1 != res2)
-    {
-      printf ("fesetround (FE_TONEAREST) failed: %d, %d\n", res1, res2);
-      ++count_errors;
-    }
-#endif
-}
-
-#if FE_ALL_EXCEPT
-/* Tests for feenableexcept/fedisableexcept.  */
-static void
-feenable_test (const char *flag_name, fexcept_t fe_exc)
-{
-  int fe_exci = fe_exc;
-  double fe_excd = fe_exc;
-  int excepts;
-
-  /* First disable all exceptions.  */
-  if (fedisableexcept (FE_ALL_EXCEPT) == -1)
-    {
-      printf ("Test: fedisableexcept (FE_ALL_EXCEPT) failed\n");
-      ++count_errors;
-      /* If this fails, the other tests don't make sense.  */
-      return;
-    }
-
-  /* Test for inline macros using integer argument.  */
-  excepts = feenableexcept (fe_exci);
-  if (!EXCEPTION_ENABLE_SUPPORTED (fe_exci) && excepts == -1)
-    {
-      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
-      return;
-    }
-  if (excepts == -1)
-    {
-      printf ("Test: feenableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != 0)
-    {
-      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
-              flag_name, excepts);
-      ++count_errors;
-    }
-
-  /* And now disable the exception again.  */
-  excepts = fedisableexcept (fe_exc);
-  if (excepts == -1)
-    {
-      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != fe_exc)
-    {
-      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
-              flag_name, (unsigned int)fe_exc, excepts);
-      ++count_errors;
-    }
-
-  /* Test for inline macros using double argument.  */
-  excepts = feenableexcept (fe_excd);
-  if (!EXCEPTION_ENABLE_SUPPORTED (fe_excd) && excepts == -1)
-    {
-      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
-      return;
-    }
-  if (excepts == -1)
-    {
-      printf ("Test: feenableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != 0)
-    {
-      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
-              flag_name, excepts);
-      ++count_errors;
-    }
-
-  /* And now disable the exception again.  */
-  excepts = fedisableexcept (fe_exc);
-  if (excepts == -1)
-    {
-      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
-      ++count_errors;
-      return;
-    }
-  if (excepts != fe_exc)
-    {
-      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
-              flag_name, (unsigned int)fe_exc, excepts);
-      ++count_errors;
-    }
-}
-#endif
-
-static void
-test_feenabledisable (void)
-{
-  printf ("Tests for feenableexcepts/fedisableexcept\n");
-
-  /* We might have some exceptions still set.  */
-  feclearexcept (FE_ALL_EXCEPT);
-
-#ifdef FE_DIVBYZERO
-  feenable_test ("FE_DIVBYZERO", FE_DIVBYZERO);
-#endif
-#ifdef FE_INVALID
-  feenable_test ("FE_INVALID", FE_INVALID);
-#endif
-#ifdef FE_INEXACT
-  feenable_test ("FE_INEXACT", FE_INEXACT);
-#endif
-#ifdef FE_UNDERFLOW
-  feenable_test ("FE_UNDERFLOW", FE_UNDERFLOW);
-#endif
-#ifdef FE_OVERFLOW
-  feenable_test ("FE_OVERFLOW", FE_OVERFLOW);
-#endif
-  fesetenv (FE_DFL_ENV);
-}
-
-static int
-do_test (void)
-{
-  /* clear all exceptions and test if all are cleared  */
-  feclearexcept (FE_ALL_EXCEPT);
-  test_exceptions ("feclearexcept (FE_ALL_EXCEPT) clears all exceptions",
-                   NO_EXC);
-
-  /* raise all exceptions and test if all are raised  */
-  feraiseexcept (FE_ALL_EXCEPT);
-  if (EXCEPTION_TESTS (float))
-    test_exceptions ("feraiseexcept (FE_ALL_EXCEPT) raises all exceptions",
-		     ALL_EXC);
-
-  /* Same test, but using double as argument  */
-  feclearexcept ((double)FE_ALL_EXCEPT);
-  test_exceptions ("feclearexcept ((double)FE_ALL_EXCEPT) clears all exceptions",
-                   NO_EXC);
-
-  feraiseexcept ((double)FE_ALL_EXCEPT);
-  if (EXCEPTION_TESTS (float))
-    test_exceptions ("feraiseexcept ((double)FE_ALL_EXCEPT) raises all exceptions",
-		     ALL_EXC);
-
-  if (EXCEPTION_TESTS (float))
-    test_exceptionflag ();
-
-  test_fesetround ();
-
-  test_feenabledisable ();
-
-  return count_errors;
-}
-
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
deleted file mode 100644
index f2d095a72f..0000000000
--- a/sysdeps/powerpc/bits/fenvinline.h
+++ /dev/null
@@ -1,108 +0,0 @@
-/* Inline floating-point environment handling functions for powerpc.
-   Copyright (C) 1995-2020 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/>.  */
-
-#if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
-
-/* Inline definitions for fegetround.  */
-# define __fegetround_ISA300()						\
-  (__extension__  ({							\
-    union { double __d; unsigned long long __ll; } __u;			\
-    __asm__ __volatile__ (						\
-      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
-      : "=f" (__u.__d));						\
-    __u.__ll & 0x0000000000000003LL;					\
-  }))
-
-# define __fegetround_ISA2()						\
-  (__extension__  ({							\
-     int __fegetround_result;						\
-     __asm__ __volatile__ ("mcrfs 7,7 ; mfcr %0"			\
-			   : "=r"(__fegetround_result) : : "cr7");	\
-     __fegetround_result & 3;						\
-  }))
-
-# ifdef _ARCH_PWR9
-#  define __fegetround() __fegetround_ISA300()
-# elif defined __BUILTIN_CPU_SUPPORTS__
-#  define __fegetround()						\
-  (__glibc_likely (__builtin_cpu_supports ("arch_3_00"))		\
-   ? __fegetround_ISA300()						\
-   : __fegetround_ISA2()						\
-  )
-# else
-#  define __fegetround() __fegetround_ISA2()
-# endif
-
-# define fegetround() __fegetround ()
-
-# ifndef __NO_MATH_INLINES
-
-/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
-#  if !__GNUC_PREREQ(9, 0)
-/* The weird 'i#*X' constraints on the following suppress a gcc
-   warning when __excepts is not a constant.  Otherwise, they mean the
-   same as just plain 'i'.  This warning only happens in old GCC
-   versions (gcc 3 or less).  Otherwise plain 'i' works fine.  */
-#   define __MTFSB0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b))
-#   define __MTFSB1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b))
-#  else
-#   define __MTFSB0(__b) __builtin_mtfsb0 (__b)
-#   define __MTFSB1(__b) __builtin_mtfsb1 (__b)
-#  endif
-
-#  if __GNUC_PREREQ(3, 4)
-
-#include <sys/param.h>
-
-/* Inline definition for feraiseexcept.  */
-#   define feraiseexcept(__excepts) \
-  (__extension__  ({ 							      \
-    int __e = __excepts;						      \
-    int __ret = 0;							      \
-    if (__builtin_constant_p (__e)					      \
-        && __builtin_popcount (__e) == 1				      \
-        && __e != FE_INVALID)						      \
-      {									      \
-	__MTFSB1 ((__builtin_clz (__e)));				      \
-      }									      \
-    else								      \
-      __ret = feraiseexcept (__e);					      \
-    __ret;								      \
-  }))
-
-/* Inline definition for feclearexcept.  */
-#   define feclearexcept(__excepts) \
-  (__extension__  ({ 							      \
-    int __e = __excepts;						      \
-    int __ret = 0;							      \
-    if (__builtin_constant_p (__e)					      \
-        && __builtin_popcount (__e) == 1				      \
-        && __e != FE_INVALID)						      \
-      {									      \
-	__MTFSB0 ((__builtin_clz (__e)));				      \
-      }									      \
-    else								      \
-      __ret = feclearexcept (__e);					      \
-    __ret;								      \
-  }))
-
-#  endif /* __GNUC_PREREQ(3, 4).  */
-
-# endif /* !__NO_MATH_INLINES.  */
-
-#endif /* __GNUC__ && !_SOFT_FLOAT && !__NO_FPRS__ */
diff --git a/sysdeps/powerpc/fpu/fegetround.c b/sysdeps/powerpc/fpu/fegetround.c
index 00b4462624..9d7762f08b 100644
--- a/sysdeps/powerpc/fpu/fegetround.c
+++ b/sysdeps/powerpc/fpu/fegetround.c
@@ -21,10 +21,8 @@
 int
 (__fegetround) (void)
 {
-  return __fegetround();
+  return __fegetround_inline ();
 }
-#undef fegetround
-#undef __fegetround
 libm_hidden_def (__fegetround)
 weak_alias (__fegetround, fegetround)
 libm_hidden_weak (fegetround)
diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
index e888c6621c..09dbd3e2df 100644
--- a/sysdeps/powerpc/fpu/fenv_libc.h
+++ b/sysdeps/powerpc/fpu/fenv_libc.h
@@ -68,6 +68,14 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
     __fr;								\
   })
 
+#define __fe_mffsl()							\
+  ({register fenv_union_t __fr;						\
+    __asm__ __volatile__ (						\
+      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
+      : "=f" (__fr.fenv));						\
+    __fr.l & 0x3;							\
+  })
+
 #define __fe_mffscrn(rn)						\
   ({register fenv_union_t __fr;						\
     if (__builtin_constant_p (rn))					\
@@ -144,6 +152,20 @@ typedef union
   unsigned long long l;
 } fenv_union_t;
 
+static inline int
+__fegetround_inline (void)
+{
+#ifdef _ARCH_PWR9
+  return __fe_mffsl ();
+#else
+  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
+    return __fe_mffsl ();
+
+  fenv_union_t fe;
+  fe.fenv = fegetenv_register ();
+  return fe.l & 0x3;
+#endif
+}
 
 static inline int
 __fesetround_inline (int round)
diff --git a/sysdeps/powerpc/fpu/fraiseexcpt.c b/sysdeps/powerpc/fpu/fraiseexcpt.c
index daa13de500..1bcd31ae14 100644
--- a/sysdeps/powerpc/fpu/fraiseexcpt.c
+++ b/sysdeps/powerpc/fpu/fraiseexcpt.c
@@ -18,7 +18,6 @@
 
 #include <fenv_libc.h>
 
-#undef feraiseexcept
 int
 __feraiseexcept (int excepts)
 {
diff --git a/sysdeps/powerpc/nofpu/fraiseexcpt.c b/sysdeps/powerpc/nofpu/fraiseexcpt.c
index 2193a604e2..e60ed68048 100644
--- a/sysdeps/powerpc/nofpu/fraiseexcpt.c
+++ b/sysdeps/powerpc/nofpu/fraiseexcpt.c
@@ -21,7 +21,6 @@
 #include "soft-supp.h"
 #include <signal.h>
 
-#undef feraiseexcept
 int
 __feraiseexcept (int x)
 {


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-10 11:29 ` Florian Weimer
@ 2020-03-10 16:58   ` Adhemerval Zanella via Libc-alpha
  2020-03-10 22:54   ` Joseph Myers
  1 sibling, 0 replies; 14+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-03-10 16:58 UTC (permalink / raw
  To: Florian Weimer; +Cc: libc-alpha



On 10/03/2020 08:29, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
>> patch removes the fenvinline.h on all architectures.  Currently
>> only powerpc implements some optimizations.  This kind of optimization
>> is better implemented by the compiler (which handles the architecture
>> ISA transparently).
>>
>> Also, for the specific optimized powerpc implementation the code is
>> becoming convoluted and these micro-optimization are hardly wildly
>> used, even more being a possible hotspot in realword cases
>> (non-default rounding are used only on specific cases and exception
>> handling are done most likely only on errors path).  Only x86
>> implements similar optimization (on fenv.h) also indicates that
>> these should no be on libc.
> 
> Wasn't the idea to make this change only after the GCC optimizations
> have been implemented?
> 

That was Tulio suggestion, but I think this fix is orthogonal because
these fenv optimizations shows similar issues as for string{2,3}.h:

  - They are no scalable, specially for architecture that provides
    different floating-point environment (such as hard and soft
    floating point such as ARM) or even hardware revision that aims
    to improve some performance deficiencies in initial revisions
    (such as POWER with ISA 3.0).

  - Newer code avoid setting exceptions directly since they are 
    costly and set on error case.  For instance, the new generic
    logf, log2f and powf implementation that do not support SVID compat.

  - Exceptions handling are hardly a hotspot, usually it is set in
    code cold path.  This is also likely for non-default rouding
    mode (since set/reset usually requires flush the pipeline on
    most FPU implementation).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-10 11:29 ` Florian Weimer
  2020-03-10 16:58   ` Adhemerval Zanella via Libc-alpha
@ 2020-03-10 22:54   ` Joseph Myers
  2020-03-16 18:09     ` Adhemerval Zanella via Libc-alpha
  1 sibling, 1 reply; 14+ messages in thread
From: Joseph Myers @ 2020-03-10 22:54 UTC (permalink / raw
  To: Florian Weimer; +Cc: libc-alpha

On Tue, 10 Mar 2020, Florian Weimer wrote:

> Wasn't the idea to make this change only after the GCC optimizations
> have been implemented?

What I did for bits/mathinline.h was file GCC bugs for the optimizations 
being removed (if not already in GCC), but not wait for them to be fixed.

(The only reason bits/mathinline.h now exists at all is that the m68k 
version is tied into the build of the out-of-line functions for m68k, 
apart from any cases where GCC might not currently know how to inline the 
functions for m68k.  That dependency on bits/mathinline.h also means 
building glibc with -Os is broken for m68k, or was the last time I tried 
building all architectures with -Os.)

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-10 22:54   ` Joseph Myers
@ 2020-03-16 18:09     ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 14+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-03-16 18:09 UTC (permalink / raw
  To: Joseph Myers, Florian Weimer; +Cc: libc-alpha



On 10/03/2020 19:54, Joseph Myers wrote:
> On Tue, 10 Mar 2020, Florian Weimer wrote:
> 
>> Wasn't the idea to make this change only after the GCC optimizations
>> have been implemented?
> 
> What I did for bits/mathinline.h was file GCC bugs for the optimizations 
> being removed (if not already in GCC), but not wait for them to be fixed.
> 

Alright, I have opened both 

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94193
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94194

And updated the commit message locally to reference it.

> (The only reason bits/mathinline.h now exists at all is that the m68k 
> version is tied into the build of the out-of-line functions for m68k, 
> apart from any cases where GCC might not currently know how to inline the 
> functions for m68k.  That dependency on bits/mathinline.h also means 
> building glibc with -Os is broken for m68k, or was the last time I tried 
> building all architectures with -Os.)
> 

For the m68k case I think moving the required macros from
mathinline.h to mathimpl.h should enable the build (and it
should make Os work as well).

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-10 16:51   ` Adhemerval Zanella
@ 2020-03-27 19:33     ` Adhemerval Zanella via Libc-alpha
  2020-03-27 21:43     ` Paul E Murphy via Libc-alpha
  1 sibling, 0 replies; 14+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-03-27 19:33 UTC (permalink / raw
  To: Paul E Murphy, libc-alpha



On 10/03/2020 13:51, Adhemerval Zanella wrote:
> 
> 
> On 09/03/2020 17:19, Paul E Murphy wrote:
>>
>>
>> On 3/9/20 1:32 PM, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>>    - Mention on commit message x86 also exports a similar optimization,
>>>      but on a different header.
>>>
>>> -- 
>>>
>>> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
>>> patch removes the fenvinline.h on all architectures.  Currently
>>> only powerpc implements some optimizations.  This kind of optimization
>>> is better implemented by the compiler (which handles the architecture
>>> ISA transparently).
>>>
>>> Also, for the specific optimized powerpc implementation the code is
>>> becoming convoluted and these micro-optimization are hardly wildly
>>> used, even more being a possible hotspot in realword cases
>>> (non-default rounding are used only on specific cases and exception
>>> handling are done most likely only on errors path).  Only x86
>>> implements similar optimization (on fenv.h) also indicates that
>>> these should no be on libc.
>>>
>>> The math/test-fenv already covers all math/test-fenvinline tests,
>>> so it is safe to remove it.
>>>
>>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>>> ---
>>>   bits/fenvinline.h                 |   8 -
>>>   math/Makefile                     |   4 +-
>>>   math/fenv.h                       |   4 -
>>>   math/test-fenvinline.c            | 354 ------------------------------
>>>   sysdeps/powerpc/bits/fenvinline.h | 108 ---------
>>>   5 files changed, 2 insertions(+), 476 deletions(-)
>>>   delete mode 100644 bits/fenvinline.h
>>>   delete mode 100644 math/test-fenvinline.c
>>>   delete mode 100644 sysdeps/powerpc/bits/fenvinline.h
> 
> Indeed, I misread the failures on powerpc64le-linux-gnu.  Below it is
> an updated patch with the fegetround optimization moved to an internal
> header.

Ping.

> 
> --
> 
> diff --git a/bits/fenvinline.h b/bits/fenvinline.h
> deleted file mode 100644
> index 42f77b5618..0000000000
> --- a/bits/fenvinline.h
> +++ /dev/null
> @@ -1,8 +0,0 @@
> -/* This file provides inline versions of floating-pint environment
> -   handling functions.  If there were any.  */
> -
> -#ifndef __NO_MATH_INLINES
> -
> -/* Here is where the code would go.  */
> -
> -#endif
> diff --git a/math/Makefile b/math/Makefile
> index 84a8b94c74..f916594a51 100644
> --- a/math/Makefile
> +++ b/math/Makefile
> @@ -24,7 +24,7 @@ include ../Makeconfig
>  # Installed header files.
>  headers		:= math.h bits/mathcalls.h bits/mathinline.h \
>  		   fpu_control.h complex.h bits/cmathcalls.h fenv.h \
> -		   bits/fenv.h bits/fenvinline.h bits/mathdef.h tgmath.h \
> +		   bits/fenv.h bits/mathdef.h tgmath.h \
>  		   bits/math-vector.h finclude/math-vector-fortran.h \
>  		   bits/libm-simd-decl-stubs.h bits/iscanonical.h \
>  		   bits/flt-eval-method.h bits/fp-fast.h bits/fp-logb.h \
> @@ -233,7 +233,7 @@ tests = test-matherr-3 test-fenv basic-test \
>  	test-misc test-fpucw test-fpucw-ieee tst-definitions test-tgmath \
>  	test-tgmath-ret bug-nextafter bug-nexttoward bug-tgmath1 \
>  	test-tgmath-int test-tgmath2 test-powl tst-CMPLX tst-CMPLX2 test-snan \
> -	test-fenv-tls test-fenv-preserve test-fenv-return test-fenvinline \
> +	test-fenv-tls test-fenv-preserve test-fenv-return \
>  	test-nearbyint-except test-fenv-clear \
>  	test-nearbyint-except-2 test-signgam-uchar test-signgam-uchar-init \
>  	test-signgam-uint test-signgam-uint-init test-signgam-ullong \
> diff --git a/math/fenv.h b/math/fenv.h
> index 6cad1d3575..e6b9578d6c 100644
> --- a/math/fenv.h
> +++ b/math/fenv.h
> @@ -140,10 +140,6 @@ extern int fegetmode (femode_t *__modep) __THROW;
>  extern int fesetmode (const femode_t *__modep) __THROW;
>  #endif
>  
> -/* Include optimization.  */
> -#ifdef __OPTIMIZE__
> -# include <bits/fenvinline.h>
> -#endif
>  
>  /* NaN support.  */
>  
> diff --git a/math/test-fenvinline.c b/math/test-fenvinline.c
> deleted file mode 100644
> index 0e5d361fff..0000000000
> --- a/math/test-fenvinline.c
> +++ /dev/null
> @@ -1,354 +0,0 @@
> -/* Test for fenv inline implementations.
> -   Copyright (C) 2015-2020 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 _GNU_SOURCE
> -# define _GNU_SOURCE
> -#endif
> -
> -/* To make sure the fenv inline function are used.  */
> -#undef __NO_MATH_INLINES
> -
> -#include <fenv.h>
> -#include <stdio.h>
> -#include <math-tests.h>
> -
> -/*
> -  Since not all architectures might define all exceptions, we define
> -  a private set and map accordingly.
> -*/
> -#define NO_EXC 0
> -#define INEXACT_EXC 0x1
> -#define DIVBYZERO_EXC 0x2
> -#define UNDERFLOW_EXC 0x04
> -#define OVERFLOW_EXC 0x08
> -#define INVALID_EXC 0x10
> -#define ALL_EXC \
> -        (INEXACT_EXC | DIVBYZERO_EXC | UNDERFLOW_EXC | OVERFLOW_EXC \
> -         | INVALID_EXC)
> -static int count_errors;
> -
> -#if FE_ALL_EXCEPT
> -static void
> -test_single_exception_fp_int (int exception,
> -			      int exc_flag,
> -			      int fe_flag,
> -			      const char *flag_name)
> -{
> -  if (exception & exc_flag)
> -    {
> -      if (fetestexcept (fe_flag))
> -        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
> -      else
> -        {
> -          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
> -          ++count_errors;
> -        }
> -    }
> -  else
> -    {
> -      if (fetestexcept (fe_flag))
> -        {
> -          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
> -          ++count_errors;
> -        }
> -      else
> -        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
> -    }
> -}
> -/* Test whether a given exception was raised.  */
> -static void
> -test_single_exception_fp_double (int exception,
> -				 int exc_flag,
> -				 double fe_flag,
> -				 const char *flag_name)
> -{
> -  if (exception & exc_flag)
> -    {
> -      if (fetestexcept (fe_flag))
> -        printf ("  Pass: Exception \"%s\" is set\n", flag_name);
> -      else
> -        {
> -          printf ("  Fail: Exception \"%s\" is not set\n", flag_name);
> -          ++count_errors;
> -        }
> -    }
> -  else
> -    {
> -      if (fetestexcept (fe_flag))
> -        {
> -          printf ("  Fail: Exception \"%s\" is set\n", flag_name);
> -          ++count_errors;
> -        }
> -      else
> -        printf ("  Pass: Exception \"%s\" is not set\n", flag_name);
> -    }
> -}
> -#endif
> -
> -static void
> -test_exceptions (const char *test_name, int exception)
> -{
> -  printf ("Test: %s\n", test_name);
> -#ifdef FE_DIVBYZERO
> -  test_single_exception_fp_double (exception, DIVBYZERO_EXC, FE_DIVBYZERO,
> -				   "DIVBYZERO");
> -#endif
> -#ifdef FE_INVALID
> -  test_single_exception_fp_double (exception, INVALID_EXC, FE_INVALID,
> -				   "INVALID");
> -#endif
> -#ifdef FE_INEXACT
> -  test_single_exception_fp_double (exception, INEXACT_EXC, FE_INEXACT,
> -				   "INEXACT");
> -#endif
> -#ifdef FE_UNDERFLOW
> -  test_single_exception_fp_double (exception, UNDERFLOW_EXC, FE_UNDERFLOW,
> -				   "UNDERFLOW");
> -#endif
> -#ifdef FE_OVERFLOW
> -  test_single_exception_fp_double (exception, OVERFLOW_EXC, FE_OVERFLOW,
> -				   "OVERFLOW");
> -#endif
> -}
> -
> -static void
> -test_exceptionflag (void)
> -{
> -  printf ("Test: fegetexceptionflag (FE_ALL_EXCEPT)\n");
> -#if FE_ALL_EXCEPT
> -  fexcept_t excepts;
> -
> -  feclearexcept (FE_ALL_EXCEPT);
> -
> -  feraiseexcept (FE_INVALID);
> -  fegetexceptflag (&excepts, FE_ALL_EXCEPT);
> -
> -  feclearexcept (FE_ALL_EXCEPT);
> -  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
> -
> -  fesetexceptflag (&excepts, FE_ALL_EXCEPT);
> -
> -  test_single_exception_fp_int (INVALID_EXC, INVALID_EXC, FE_INVALID,
> -				"INVALID (int)");
> -  test_single_exception_fp_int (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
> -				"OVERFLOW (int)");
> -  test_single_exception_fp_int (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
> -				"INEXACT (int)");
> -
> -  /* Same test, but using double as argument  */
> -  feclearexcept (FE_ALL_EXCEPT);
> -
> -  feraiseexcept (FE_INVALID);
> -  fegetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
> -
> -  feclearexcept (FE_ALL_EXCEPT);
> -  feraiseexcept (FE_OVERFLOW | FE_INEXACT);
> -
> -  fesetexceptflag (&excepts, (double)FE_ALL_EXCEPT);
> -
> -  test_single_exception_fp_double (INVALID_EXC, INVALID_EXC, FE_INVALID,
> -				   "INVALID (double)");
> -  test_single_exception_fp_double (INVALID_EXC, OVERFLOW_EXC, FE_OVERFLOW,
> -				   "OVERFLOW (double)");
> -  test_single_exception_fp_double (INVALID_EXC, INEXACT_EXC, FE_INEXACT,
> -				   "INEXACT (double)");
> -#endif
> -}
> -
> -static void
> -test_fesetround (void)
> -{
> -#if defined FE_TONEAREST && defined FE_TOWARDZERO
> -  int res1;
> -  int res2;
> -
> -  printf ("Tests for fesetround\n");
> -
> -  /* The fesetround should not itself cause the test to fail, however it
> -     should either succeed for both 'int' and 'double' argument, or fail
> -     for both.  */
> -  res1 = fesetround ((int) FE_TOWARDZERO);
> -  res2 = fesetround ((double) FE_TOWARDZERO);
> -  if (res1 != res2)
> -    {
> -      printf ("fesetround (FE_TOWARDZERO) failed: %d, %d\n", res1, res2);
> -      ++count_errors;
> -    }
> -
> -  res1 = fesetround ((int) FE_TONEAREST);
> -  res2 = fesetround ((double) FE_TONEAREST);
> -  if (res1 != res2)
> -    {
> -      printf ("fesetround (FE_TONEAREST) failed: %d, %d\n", res1, res2);
> -      ++count_errors;
> -    }
> -#endif
> -}
> -
> -#if FE_ALL_EXCEPT
> -/* Tests for feenableexcept/fedisableexcept.  */
> -static void
> -feenable_test (const char *flag_name, fexcept_t fe_exc)
> -{
> -  int fe_exci = fe_exc;
> -  double fe_excd = fe_exc;
> -  int excepts;
> -
> -  /* First disable all exceptions.  */
> -  if (fedisableexcept (FE_ALL_EXCEPT) == -1)
> -    {
> -      printf ("Test: fedisableexcept (FE_ALL_EXCEPT) failed\n");
> -      ++count_errors;
> -      /* If this fails, the other tests don't make sense.  */
> -      return;
> -    }
> -
> -  /* Test for inline macros using integer argument.  */
> -  excepts = feenableexcept (fe_exci);
> -  if (!EXCEPTION_ENABLE_SUPPORTED (fe_exci) && excepts == -1)
> -    {
> -      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
> -      return;
> -    }
> -  if (excepts == -1)
> -    {
> -      printf ("Test: feenableexcept (%s) failed\n", flag_name);
> -      ++count_errors;
> -      return;
> -    }
> -  if (excepts != 0)
> -    {
> -      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
> -              flag_name, excepts);
> -      ++count_errors;
> -    }
> -
> -  /* And now disable the exception again.  */
> -  excepts = fedisableexcept (fe_exc);
> -  if (excepts == -1)
> -    {
> -      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
> -      ++count_errors;
> -      return;
> -    }
> -  if (excepts != fe_exc)
> -    {
> -      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
> -              flag_name, (unsigned int)fe_exc, excepts);
> -      ++count_errors;
> -    }
> -
> -  /* Test for inline macros using double argument.  */
> -  excepts = feenableexcept (fe_excd);
> -  if (!EXCEPTION_ENABLE_SUPPORTED (fe_excd) && excepts == -1)
> -    {
> -      printf ("Test: not testing feenableexcept, it isn't implemented.\n");
> -      return;
> -    }
> -  if (excepts == -1)
> -    {
> -      printf ("Test: feenableexcept (%s) failed\n", flag_name);
> -      ++count_errors;
> -      return;
> -    }
> -  if (excepts != 0)
> -    {
> -      printf ("Test: feenableexcept (%s) failed, return should be 0, is %x\n",
> -              flag_name, excepts);
> -      ++count_errors;
> -    }
> -
> -  /* And now disable the exception again.  */
> -  excepts = fedisableexcept (fe_exc);
> -  if (excepts == -1)
> -    {
> -      printf ("Test: fedisableexcept (%s) failed\n", flag_name);
> -      ++count_errors;
> -      return;
> -    }
> -  if (excepts != fe_exc)
> -    {
> -      printf ("Test: fedisableexcept (%s) failed, return should be 0x%x, is 0x%x\n",
> -              flag_name, (unsigned int)fe_exc, excepts);
> -      ++count_errors;
> -    }
> -}
> -#endif
> -
> -static void
> -test_feenabledisable (void)
> -{
> -  printf ("Tests for feenableexcepts/fedisableexcept\n");
> -
> -  /* We might have some exceptions still set.  */
> -  feclearexcept (FE_ALL_EXCEPT);
> -
> -#ifdef FE_DIVBYZERO
> -  feenable_test ("FE_DIVBYZERO", FE_DIVBYZERO);
> -#endif
> -#ifdef FE_INVALID
> -  feenable_test ("FE_INVALID", FE_INVALID);
> -#endif
> -#ifdef FE_INEXACT
> -  feenable_test ("FE_INEXACT", FE_INEXACT);
> -#endif
> -#ifdef FE_UNDERFLOW
> -  feenable_test ("FE_UNDERFLOW", FE_UNDERFLOW);
> -#endif
> -#ifdef FE_OVERFLOW
> -  feenable_test ("FE_OVERFLOW", FE_OVERFLOW);
> -#endif
> -  fesetenv (FE_DFL_ENV);
> -}
> -
> -static int
> -do_test (void)
> -{
> -  /* clear all exceptions and test if all are cleared  */
> -  feclearexcept (FE_ALL_EXCEPT);
> -  test_exceptions ("feclearexcept (FE_ALL_EXCEPT) clears all exceptions",
> -                   NO_EXC);
> -
> -  /* raise all exceptions and test if all are raised  */
> -  feraiseexcept (FE_ALL_EXCEPT);
> -  if (EXCEPTION_TESTS (float))
> -    test_exceptions ("feraiseexcept (FE_ALL_EXCEPT) raises all exceptions",
> -		     ALL_EXC);
> -
> -  /* Same test, but using double as argument  */
> -  feclearexcept ((double)FE_ALL_EXCEPT);
> -  test_exceptions ("feclearexcept ((double)FE_ALL_EXCEPT) clears all exceptions",
> -                   NO_EXC);
> -
> -  feraiseexcept ((double)FE_ALL_EXCEPT);
> -  if (EXCEPTION_TESTS (float))
> -    test_exceptions ("feraiseexcept ((double)FE_ALL_EXCEPT) raises all exceptions",
> -		     ALL_EXC);
> -
> -  if (EXCEPTION_TESTS (float))
> -    test_exceptionflag ();
> -
> -  test_fesetround ();
> -
> -  test_feenabledisable ();
> -
> -  return count_errors;
> -}
> -
> -#define TEST_FUNCTION do_test ()
> -#include "../test-skeleton.c"
> diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h
> deleted file mode 100644
> index f2d095a72f..0000000000
> --- a/sysdeps/powerpc/bits/fenvinline.h
> +++ /dev/null
> @@ -1,108 +0,0 @@
> -/* Inline floating-point environment handling functions for powerpc.
> -   Copyright (C) 1995-2020 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/>.  */
> -
> -#if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__
> -
> -/* Inline definitions for fegetround.  */
> -# define __fegetround_ISA300()						\
> -  (__extension__  ({							\
> -    union { double __d; unsigned long long __ll; } __u;			\
> -    __asm__ __volatile__ (						\
> -      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
> -      : "=f" (__u.__d));						\
> -    __u.__ll & 0x0000000000000003LL;					\
> -  }))
> -
> -# define __fegetround_ISA2()						\
> -  (__extension__  ({							\
> -     int __fegetround_result;						\
> -     __asm__ __volatile__ ("mcrfs 7,7 ; mfcr %0"			\
> -			   : "=r"(__fegetround_result) : : "cr7");	\
> -     __fegetround_result & 3;						\
> -  }))
> -
> -# ifdef _ARCH_PWR9
> -#  define __fegetround() __fegetround_ISA300()
> -# elif defined __BUILTIN_CPU_SUPPORTS__
> -#  define __fegetround()						\
> -  (__glibc_likely (__builtin_cpu_supports ("arch_3_00"))		\
> -   ? __fegetround_ISA300()						\
> -   : __fegetround_ISA2()						\
> -  )
> -# else
> -#  define __fegetround() __fegetround_ISA2()
> -# endif
> -
> -# define fegetround() __fegetround ()
> -
> -# ifndef __NO_MATH_INLINES
> -
> -/* Builtins to mtfsb0 and mtfsb1 was introduced on GCC 9.  */
> -#  if !__GNUC_PREREQ(9, 0)
> -/* The weird 'i#*X' constraints on the following suppress a gcc
> -   warning when __excepts is not a constant.  Otherwise, they mean the
> -   same as just plain 'i'.  This warning only happens in old GCC
> -   versions (gcc 3 or less).  Otherwise plain 'i' works fine.  */
> -#   define __MTFSB0(__b) __asm__ __volatile__ ("mtfsb0 %0" : : "i#*X" (__b))
> -#   define __MTFSB1(__b) __asm__ __volatile__ ("mtfsb1 %0" : : "i#*X" (__b))
> -#  else
> -#   define __MTFSB0(__b) __builtin_mtfsb0 (__b)
> -#   define __MTFSB1(__b) __builtin_mtfsb1 (__b)
> -#  endif
> -
> -#  if __GNUC_PREREQ(3, 4)
> -
> -#include <sys/param.h>
> -
> -/* Inline definition for feraiseexcept.  */
> -#   define feraiseexcept(__excepts) \
> -  (__extension__  ({ 							      \
> -    int __e = __excepts;						      \
> -    int __ret = 0;							      \
> -    if (__builtin_constant_p (__e)					      \
> -        && __builtin_popcount (__e) == 1				      \
> -        && __e != FE_INVALID)						      \
> -      {									      \
> -	__MTFSB1 ((__builtin_clz (__e)));				      \
> -      }									      \
> -    else								      \
> -      __ret = feraiseexcept (__e);					      \
> -    __ret;								      \
> -  }))
> -
> -/* Inline definition for feclearexcept.  */
> -#   define feclearexcept(__excepts) \
> -  (__extension__  ({ 							      \
> -    int __e = __excepts;						      \
> -    int __ret = 0;							      \
> -    if (__builtin_constant_p (__e)					      \
> -        && __builtin_popcount (__e) == 1				      \
> -        && __e != FE_INVALID)						      \
> -      {									      \
> -	__MTFSB0 ((__builtin_clz (__e)));				      \
> -      }									      \
> -    else								      \
> -      __ret = feclearexcept (__e);					      \
> -    __ret;								      \
> -  }))
> -
> -#  endif /* __GNUC_PREREQ(3, 4).  */
> -
> -# endif /* !__NO_MATH_INLINES.  */
> -
> -#endif /* __GNUC__ && !_SOFT_FLOAT && !__NO_FPRS__ */
> diff --git a/sysdeps/powerpc/fpu/fegetround.c b/sysdeps/powerpc/fpu/fegetround.c
> index 00b4462624..9d7762f08b 100644
> --- a/sysdeps/powerpc/fpu/fegetround.c
> +++ b/sysdeps/powerpc/fpu/fegetround.c
> @@ -21,10 +21,8 @@
>  int
>  (__fegetround) (void)
>  {
> -  return __fegetround();
> +  return __fegetround_inline ();
>  }
> -#undef fegetround
> -#undef __fegetround
>  libm_hidden_def (__fegetround)
>  weak_alias (__fegetround, fegetround)
>  libm_hidden_weak (fegetround)
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index e888c6621c..09dbd3e2df 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -68,6 +68,14 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
>      __fr;								\
>    })
>  
> +#define __fe_mffsl()							\
> +  ({register fenv_union_t __fr;						\
> +    __asm__ __volatile__ (						\
> +      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
> +      : "=f" (__fr.fenv));						\
> +    __fr.l & 0x3;							\
> +  })
> +
>  #define __fe_mffscrn(rn)						\
>    ({register fenv_union_t __fr;						\
>      if (__builtin_constant_p (rn))					\
> @@ -144,6 +152,20 @@ typedef union
>    unsigned long long l;
>  } fenv_union_t;
>  
> +static inline int
> +__fegetround_inline (void)
> +{
> +#ifdef _ARCH_PWR9
> +  return __fe_mffsl ();
> +#else
> +  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
> +    return __fe_mffsl ();
> +
> +  fenv_union_t fe;
> +  fe.fenv = fegetenv_register ();
> +  return fe.l & 0x3;
> +#endif
> +}
>  
>  static inline int
>  __fesetround_inline (int round)
> diff --git a/sysdeps/powerpc/fpu/fraiseexcpt.c b/sysdeps/powerpc/fpu/fraiseexcpt.c
> index daa13de500..1bcd31ae14 100644
> --- a/sysdeps/powerpc/fpu/fraiseexcpt.c
> +++ b/sysdeps/powerpc/fpu/fraiseexcpt.c
> @@ -18,7 +18,6 @@
>  
>  #include <fenv_libc.h>
>  
> -#undef feraiseexcept
>  int
>  __feraiseexcept (int excepts)
>  {
> diff --git a/sysdeps/powerpc/nofpu/fraiseexcpt.c b/sysdeps/powerpc/nofpu/fraiseexcpt.c
> index 2193a604e2..e60ed68048 100644
> --- a/sysdeps/powerpc/nofpu/fraiseexcpt.c
> +++ b/sysdeps/powerpc/nofpu/fraiseexcpt.c
> @@ -21,7 +21,6 @@
>  #include "soft-supp.h"
>  #include <signal.h>
>  
> -#undef feraiseexcept
>  int
>  __feraiseexcept (int x)
>  {
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] x86: Remove feraiseexcept optimization
  2020-03-09 18:32 ` [PATCH 2/3] x86: Remove feraiseexcept optimization Adhemerval Zanella
@ 2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
  2020-03-27 19:56     ` H.J. Lu via Libc-alpha
  0 siblings, 1 reply; 14+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-03-27 19:34 UTC (permalink / raw
  To: libc-alpha


Ping.

On 09/03/2020 15:32, Adhemerval Zanella wrote:
> Similar to fenvinline.h removal, this kind of optimization is better
> implemented by the compiler.  Also newer code avoid setting exceptions
> directly (for instance the code to make new logf, log2f and powf
> implementatation to now support SVID compat).
> 
> Checked on x86_64-linux-gnu and i686-linux-gnu.
> ---
>  sysdeps/x86/fpu/bits/fenv.h         | 54 ---------------------------
>  sysdeps/x86/fpu/include/bits/fenv.h | 57 -----------------------------
>  2 files changed, 111 deletions(-)
>  delete mode 100644 sysdeps/x86/fpu/include/bits/fenv.h
> 
> diff --git a/sysdeps/x86/fpu/bits/fenv.h b/sysdeps/x86/fpu/bits/fenv.h
> index eb78eb4b2d..6cfa5678eb 100644
> --- a/sysdeps/x86/fpu/bits/fenv.h
> +++ b/sysdeps/x86/fpu/bits/fenv.h
> @@ -114,57 +114,3 @@ femode_t;
>  /* Default floating-point control modes.  */
>  # define FE_DFL_MODE	((const femode_t *) -1L)
>  #endif
> -
> -
> -#ifdef __USE_EXTERN_INLINES
> -__BEGIN_DECLS
> -
> -/* Optimized versions.  */
> -#ifndef _LIBC
> -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
> -#endif
> -__extern_always_inline void
> -__NTH (__feraiseexcept_invalid_divbyzero (int __excepts))
> -{
> -  if ((FE_INVALID & __excepts) != 0)
> -    {
> -      /* One example of an invalid operation is 0.0 / 0.0.  */
> -      float __f = 0.0;
> -
> -# ifdef __SSE_MATH__
> -      __asm__ __volatile__ ("divss %0, %0 " : : "x" (__f));
> -# else
> -      __asm__ __volatile__ ("fdiv %%st, %%st(0); fwait"
> -			    : "=t" (__f) : "0" (__f));
> -# endif
> -      (void) &__f;
> -    }
> -  if ((FE_DIVBYZERO & __excepts) != 0)
> -    {
> -      float __f = 1.0;
> -      float __g = 0.0;
> -
> -# ifdef __SSE_MATH__
> -      __asm__ __volatile__ ("divss %1, %0" : : "x" (__f), "x" (__g));
> -# else
> -      __asm__ __volatile__ ("fdivp %%st, %%st(1); fwait"
> -			    : "=t" (__f) : "0" (__f), "u" (__g) : "st(1)");
> -# endif
> -      (void) &__f;
> -    }
> -}
> -__extern_inline int
> -__NTH (feraiseexcept (int __excepts))
> -{
> -  if (__builtin_constant_p (__excepts)
> -      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
> -    {
> -      __feraiseexcept_invalid_divbyzero (__excepts);
> -      return 0;
> -    }
> -
> -  return __feraiseexcept_renamed (__excepts);
> -}
> -
> -__END_DECLS
> -#endif
> diff --git a/sysdeps/x86/fpu/include/bits/fenv.h b/sysdeps/x86/fpu/include/bits/fenv.h
> deleted file mode 100644
> index dd3f61e9f3..0000000000
> --- a/sysdeps/x86/fpu/include/bits/fenv.h
> +++ /dev/null
> @@ -1,57 +0,0 @@
> -/* Wrapper for x86 bits/fenv.h for use when building glibc.
> -   Copyright (C) 1997-2020 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 _BITS_FENV_H
> -
> -#if defined _LIBC && defined __USE_EXTERN_INLINES
> -# if defined SHARED && !defined NO_HIDDEN && IS_IN (libm)
> -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), __GI_feraiseexcept);
> -# else
> -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
> -# endif
> -#endif
> -
> -#include_next <bits/fenv.h>
> -
> -# ifndef _ISOMAC
> -
> -/* Ensure __feraiseexcept calls in glibc are optimized the same as
> -   feraiseexcept calls.  */
> -
> -#ifdef __USE_EXTERN_INLINES
> -__BEGIN_DECLS
> -
> -extern int __REDIRECT_NTH (____feraiseexcept_renamed, (int), __feraiseexcept);
> -__extern_inline int
> -__NTH (__feraiseexcept (int __excepts))
> -{
> -  if (__builtin_constant_p (__excepts)
> -      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
> -    {
> -      __feraiseexcept_invalid_divbyzero (__excepts);
> -      return 0;
> -    }
> -
> -  return ____feraiseexcept_renamed (__excepts);
> -}
> -
> -__END_DECLS
> -#endif
> -
> -# endif /* _ISOMAC */
> -#endif /* bits/fenv.h */
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h
  2020-03-09 18:32 ` [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h Adhemerval Zanella
@ 2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
  0 siblings, 0 replies; 14+ messages in thread
From: Adhemerval Zanella via Libc-alpha @ 2020-03-27 19:34 UTC (permalink / raw
  To: libc-alpha


Ping.

On 09/03/2020 15:32, Adhemerval Zanella wrote:
> These should not be exported on installed headers.
> 
> Checked on sparc64-linux-gnu and sparcv9-linux-gnu.
> ---
>  sysdeps/sparc/fpu/bits/fenv.h    | 9 ---------
>  sysdeps/sparc/fpu/fclrexcpt.c    | 1 +
>  sysdeps/sparc/fpu/fedisblxcpt.c  | 1 +
>  sysdeps/sparc/fpu/feenablxcpt.c  | 1 +
>  sysdeps/sparc/fpu/fegetenv.c     | 1 +
>  sysdeps/sparc/fpu/fegetexcept.c  | 1 +
>  sysdeps/sparc/fpu/fegetmode.c    | 1 +
>  sysdeps/sparc/fpu/fegetround.c   | 1 +
>  sysdeps/sparc/fpu/feholdexcpt.c  | 1 +
>  sysdeps/sparc/fpu/fenv_private.h | 9 +++++++++
>  sysdeps/sparc/fpu/fesetenv.c     | 1 +
>  sysdeps/sparc/fpu/fesetexcept.c  | 1 +
>  sysdeps/sparc/fpu/fesetmode.c    | 1 +
>  sysdeps/sparc/fpu/fesetround.c   | 1 +
>  sysdeps/sparc/fpu/feupdateenv.c  | 1 +
>  sysdeps/sparc/fpu/fgetexcptflg.c | 1 +
>  sysdeps/sparc/fpu/fsetexcptflg.c | 1 +
>  sysdeps/sparc/fpu/ftestexcept.c  | 1 +
>  18 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/sysdeps/sparc/fpu/bits/fenv.h b/sysdeps/sparc/fpu/bits/fenv.h
> index 4935208a41..d34fcac4c5 100644
> --- a/sysdeps/sparc/fpu/bits/fenv.h
> +++ b/sysdeps/sparc/fpu/bits/fenv.h
> @@ -83,15 +83,6 @@ typedef unsigned long int fenv_t;
>  # define FE_NOMASK_ENV	((const fenv_t *) -2)
>  #endif
>  
> -/* For internal use only: access the fp state register.  */
> -#if __WORDSIZE == 64
> -# define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
> -# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
> -#else
> -# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
> -# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
> -#endif
> -
>  #if __GLIBC_USE (IEC_60559_BFP_EXT_C2X)
>  /* Type representing floating-point control modes.  */
>  typedef unsigned long int femode_t;
> diff --git a/sysdeps/sparc/fpu/fclrexcpt.c b/sysdeps/sparc/fpu/fclrexcpt.c
> index b11734f057..5af20d1f7a 100644
> --- a/sysdeps/sparc/fpu/fclrexcpt.c
> +++ b/sysdeps/sparc/fpu/fclrexcpt.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <shlib-compat.h>
>  
>  int
> diff --git a/sysdeps/sparc/fpu/fedisblxcpt.c b/sysdeps/sparc/fpu/fedisblxcpt.c
> index 86688ab533..9b832a82ce 100644
> --- a/sysdeps/sparc/fpu/fedisblxcpt.c
> +++ b/sysdeps/sparc/fpu/fedisblxcpt.c
> @@ -18,6 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  fedisableexcept (int excepts)
> diff --git a/sysdeps/sparc/fpu/feenablxcpt.c b/sysdeps/sparc/fpu/feenablxcpt.c
> index 647093cebc..06ec14cee5 100644
> --- a/sysdeps/sparc/fpu/feenablxcpt.c
> +++ b/sysdeps/sparc/fpu/feenablxcpt.c
> @@ -18,6 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  feenableexcept (int excepts)
> diff --git a/sysdeps/sparc/fpu/fegetenv.c b/sysdeps/sparc/fpu/fegetenv.c
> index edde6ae5b2..00c0bc72b5 100644
> --- a/sysdeps/sparc/fpu/fegetenv.c
> +++ b/sysdeps/sparc/fpu/fegetenv.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <shlib-compat.h>
>  
>  int
> diff --git a/sysdeps/sparc/fpu/fegetexcept.c b/sysdeps/sparc/fpu/fegetexcept.c
> index f549a90190..4d9746dd57 100644
> --- a/sysdeps/sparc/fpu/fegetexcept.c
> +++ b/sysdeps/sparc/fpu/fegetexcept.c
> @@ -18,6 +18,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  fegetexcept (void)
> diff --git a/sysdeps/sparc/fpu/fegetmode.c b/sysdeps/sparc/fpu/fegetmode.c
> index 18c932d520..aa160bd19a 100644
> --- a/sysdeps/sparc/fpu/fegetmode.c
> +++ b/sysdeps/sparc/fpu/fegetmode.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  fegetmode (femode_t *modep)
> diff --git a/sysdeps/sparc/fpu/fegetround.c b/sysdeps/sparc/fpu/fegetround.c
> index 1eae341fc4..6ca7d5c0dc 100644
> --- a/sysdeps/sparc/fpu/fegetround.c
> +++ b/sysdeps/sparc/fpu/fegetround.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  __fegetround (void)
> diff --git a/sysdeps/sparc/fpu/feholdexcpt.c b/sysdeps/sparc/fpu/feholdexcpt.c
> index 7a1a3e33ed..bb612402f0 100644
> --- a/sysdeps/sparc/fpu/feholdexcpt.c
> +++ b/sysdeps/sparc/fpu/feholdexcpt.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  __feholdexcept (fenv_t *envp)
> diff --git a/sysdeps/sparc/fpu/fenv_private.h b/sysdeps/sparc/fpu/fenv_private.h
> index dbd1001ccb..da7c7fe332 100644
> --- a/sysdeps/sparc/fpu/fenv_private.h
> +++ b/sysdeps/sparc/fpu/fenv_private.h
> @@ -3,6 +3,15 @@
>  
>  #include <fenv.h>
>  
> +/* For internal use only: access the fp state register.  */
> +#if __WORDSIZE == 64
> +# define __fenv_stfsr(X)   __asm__ __volatile__ ("stx %%fsr,%0" : "=m" (X))
> +# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ldx %0,%%fsr" : : "m" (X))
> +#else
> +# define __fenv_stfsr(X)   __asm__ __volatile__ ("st %%fsr,%0" : "=m" (X))
> +# define __fenv_ldfsr(X)   __asm__ __volatile__ ("ld %0,%%fsr" : : "m" (X))
> +#endif
> +
>  static __always_inline void
>  libc_feholdexcept (fenv_t *e)
>  {
> diff --git a/sysdeps/sparc/fpu/fesetenv.c b/sysdeps/sparc/fpu/fesetenv.c
> index 82c03c6760..d536abd344 100644
> --- a/sysdeps/sparc/fpu/fesetenv.c
> +++ b/sysdeps/sparc/fpu/fesetenv.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <shlib-compat.h>
>  
>  int
> diff --git a/sysdeps/sparc/fpu/fesetexcept.c b/sysdeps/sparc/fpu/fesetexcept.c
> index 6740ece5b4..fbc21c0477 100644
> --- a/sysdeps/sparc/fpu/fesetexcept.c
> +++ b/sysdeps/sparc/fpu/fesetexcept.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  fesetexcept (int excepts)
> diff --git a/sysdeps/sparc/fpu/fesetmode.c b/sysdeps/sparc/fpu/fesetmode.c
> index 6fe5d337ad..24148e0fd3 100644
> --- a/sysdeps/sparc/fpu/fesetmode.c
> +++ b/sysdeps/sparc/fpu/fesetmode.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <fpu_control.h>
>  
>  #define FPU_CONTROL_BITS 0xcfc00000UL
> diff --git a/sysdeps/sparc/fpu/fesetround.c b/sysdeps/sparc/fpu/fesetround.c
> index 9a944322d7..b259474d2c 100644
> --- a/sysdeps/sparc/fpu/fesetround.c
> +++ b/sysdeps/sparc/fpu/fesetround.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  __fesetround (int round)
> diff --git a/sysdeps/sparc/fpu/feupdateenv.c b/sysdeps/sparc/fpu/feupdateenv.c
> index 7e2399bfa2..7721f822ea 100644
> --- a/sysdeps/sparc/fpu/feupdateenv.c
> +++ b/sysdeps/sparc/fpu/feupdateenv.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <shlib-compat.h>
>  
>  int
> diff --git a/sysdeps/sparc/fpu/fgetexcptflg.c b/sysdeps/sparc/fpu/fgetexcptflg.c
> index f95d9bbf1b..ab8fa1bb76 100644
> --- a/sysdeps/sparc/fpu/fgetexcptflg.c
> +++ b/sysdeps/sparc/fpu/fgetexcptflg.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <shlib-compat.h>
>  
>  int
> diff --git a/sysdeps/sparc/fpu/fsetexcptflg.c b/sysdeps/sparc/fpu/fsetexcptflg.c
> index 077dfc9953..34eb789a94 100644
> --- a/sysdeps/sparc/fpu/fsetexcptflg.c
> +++ b/sysdeps/sparc/fpu/fsetexcptflg.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  #include <math.h>
>  #include <shlib-compat.h>
>  
> diff --git a/sysdeps/sparc/fpu/ftestexcept.c b/sysdeps/sparc/fpu/ftestexcept.c
> index a8c8e06ef6..44367ab4fd 100644
> --- a/sysdeps/sparc/fpu/ftestexcept.c
> +++ b/sysdeps/sparc/fpu/ftestexcept.c
> @@ -17,6 +17,7 @@
>     <https://www.gnu.org/licenses/>.  */
>  
>  #include <fenv.h>
> +#include <fenv_private.h>
>  
>  int
>  fetestexcept (int excepts)
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] x86: Remove feraiseexcept optimization
  2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
@ 2020-03-27 19:56     ` H.J. Lu via Libc-alpha
  0 siblings, 0 replies; 14+ messages in thread
From: H.J. Lu via Libc-alpha @ 2020-03-27 19:56 UTC (permalink / raw
  To: Adhemerval Zanella; +Cc: GNU C Library

On Fri, Mar 27, 2020 at 12:35 PM Adhemerval Zanella via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
>
> Ping.
>
> On 09/03/2020 15:32, Adhemerval Zanella wrote:
> > Similar to fenvinline.h removal, this kind of optimization is better
> > implemented by the compiler.  Also newer code avoid setting exceptions
> > directly (for instance the code to make new logf, log2f and powf
> > implementatation to now support SVID compat).
> >
> > Checked on x86_64-linux-gnu and i686-linux-gnu.
> > ---
> >  sysdeps/x86/fpu/bits/fenv.h         | 54 ---------------------------
> >  sysdeps/x86/fpu/include/bits/fenv.h | 57 -----------------------------
> >  2 files changed, 111 deletions(-)
> >  delete mode 100644 sysdeps/x86/fpu/include/bits/fenv.h
> >
> > diff --git a/sysdeps/x86/fpu/bits/fenv.h b/sysdeps/x86/fpu/bits/fenv.h
> > index eb78eb4b2d..6cfa5678eb 100644
> > --- a/sysdeps/x86/fpu/bits/fenv.h
> > +++ b/sysdeps/x86/fpu/bits/fenv.h
> > @@ -114,57 +114,3 @@ femode_t;
> >  /* Default floating-point control modes.  */
> >  # define FE_DFL_MODE ((const femode_t *) -1L)
> >  #endif
> > -
> > -
> > -#ifdef __USE_EXTERN_INLINES
> > -__BEGIN_DECLS
> > -
> > -/* Optimized versions.  */
> > -#ifndef _LIBC
> > -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
> > -#endif
> > -__extern_always_inline void
> > -__NTH (__feraiseexcept_invalid_divbyzero (int __excepts))
> > -{
> > -  if ((FE_INVALID & __excepts) != 0)
> > -    {
> > -      /* One example of an invalid operation is 0.0 / 0.0.  */
> > -      float __f = 0.0;
> > -
> > -# ifdef __SSE_MATH__
> > -      __asm__ __volatile__ ("divss %0, %0 " : : "x" (__f));
> > -# else
> > -      __asm__ __volatile__ ("fdiv %%st, %%st(0); fwait"
> > -                         : "=t" (__f) : "0" (__f));
> > -# endif
> > -      (void) &__f;
> > -    }
> > -  if ((FE_DIVBYZERO & __excepts) != 0)
> > -    {
> > -      float __f = 1.0;
> > -      float __g = 0.0;
> > -
> > -# ifdef __SSE_MATH__
> > -      __asm__ __volatile__ ("divss %1, %0" : : "x" (__f), "x" (__g));
> > -# else
> > -      __asm__ __volatile__ ("fdivp %%st, %%st(1); fwait"
> > -                         : "=t" (__f) : "0" (__f), "u" (__g) : "st(1)");
> > -# endif
> > -      (void) &__f;
> > -    }
> > -}
> > -__extern_inline int
> > -__NTH (feraiseexcept (int __excepts))
> > -{
> > -  if (__builtin_constant_p (__excepts)
> > -      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
> > -    {
> > -      __feraiseexcept_invalid_divbyzero (__excepts);
> > -      return 0;
> > -    }
> > -
> > -  return __feraiseexcept_renamed (__excepts);
> > -}
> > -
> > -__END_DECLS
> > -#endif
> > diff --git a/sysdeps/x86/fpu/include/bits/fenv.h b/sysdeps/x86/fpu/include/bits/fenv.h
> > deleted file mode 100644
> > index dd3f61e9f3..0000000000
> > --- a/sysdeps/x86/fpu/include/bits/fenv.h
> > +++ /dev/null
> > @@ -1,57 +0,0 @@
> > -/* Wrapper for x86 bits/fenv.h for use when building glibc.
> > -   Copyright (C) 1997-2020 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 _BITS_FENV_H
> > -
> > -#if defined _LIBC && defined __USE_EXTERN_INLINES
> > -# if defined SHARED && !defined NO_HIDDEN && IS_IN (libm)
> > -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), __GI_feraiseexcept);
> > -# else
> > -extern int __REDIRECT_NTH (__feraiseexcept_renamed, (int), feraiseexcept);
> > -# endif
> > -#endif
> > -
> > -#include_next <bits/fenv.h>
> > -
> > -# ifndef _ISOMAC
> > -
> > -/* Ensure __feraiseexcept calls in glibc are optimized the same as
> > -   feraiseexcept calls.  */
> > -
> > -#ifdef __USE_EXTERN_INLINES
> > -__BEGIN_DECLS
> > -
> > -extern int __REDIRECT_NTH (____feraiseexcept_renamed, (int), __feraiseexcept);
> > -__extern_inline int
> > -__NTH (__feraiseexcept (int __excepts))
> > -{
> > -  if (__builtin_constant_p (__excepts)
> > -      && (__excepts & ~(FE_INVALID | FE_DIVBYZERO)) == 0)
> > -    {
> > -      __feraiseexcept_invalid_divbyzero (__excepts);
> > -      return 0;
> > -    }
> > -
> > -  return ____feraiseexcept_renamed (__excepts);
> > -}
> > -
> > -__END_DECLS
> > -#endif
> > -
> > -# endif /* _ISOMAC */
> > -#endif /* bits/fenv.h */
> >

LGTM.

Thanks.

-- 
H.J.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] math: Remove fenvinline.h
  2020-03-10 16:51   ` Adhemerval Zanella
  2020-03-27 19:33     ` Adhemerval Zanella via Libc-alpha
@ 2020-03-27 21:43     ` Paul E Murphy via Libc-alpha
  1 sibling, 0 replies; 14+ messages in thread
From: Paul E Murphy via Libc-alpha @ 2020-03-27 21:43 UTC (permalink / raw
  To: Adhemerval Zanella, libc-alpha



On 3/10/20 11:51 AM, Adhemerval Zanella wrote:
> 
> 
> On 09/03/2020 17:19, Paul E Murphy wrote:
>>
>>
>> On 3/9/20 1:32 PM, Adhemerval Zanella wrote:
>>> Changes from previous version:
>>>
>>>     - Mention on commit message x86 also exports a similar optimization,
>>>       but on a different header.
>>>
>>> -- 
>>>
>>> Similar to string2.h (18b10de7ce) and string3.h (09a596cc2c) this
>>> patch removes the fenvinline.h on all architectures.  Currently
>>> only powerpc implements some optimizations.  This kind of optimization
>>> is better implemented by the compiler (which handles the architecture
>>> ISA transparently).
>>>
>>> Also, for the specific optimized powerpc implementation the code is
>>> becoming convoluted and these micro-optimization are hardly wildly
>>> used, even more being a possible hotspot in realword cases
>>> (non-default rounding are used only on specific cases and exception
>>> handling are done most likely only on errors path).  Only x86
>>> implements similar optimization (on fenv.h) also indicates that
>>> these should no be on libc.
>>>
>>> The math/test-fenv already covers all math/test-fenvinline tests,
>>> so it is safe to remove it.
>>>
>>> Checked on x86_64-linux-gnu and powerpc64le-linux-gnu.
>>> ---
>>>    bits/fenvinline.h                 |   8 -
>>>    math/Makefile                     |   4 +-
>>>    math/fenv.h                       |   4 -
>>>    math/test-fenvinline.c            | 354 ------------------------------
>>>    sysdeps/powerpc/bits/fenvinline.h | 108 ---------
>>>    5 files changed, 2 insertions(+), 476 deletions(-)
>>>    delete mode 100644 bits/fenvinline.h
>>>    delete mode 100644 math/test-fenvinline.c
>>>    delete mode 100644 sysdeps/powerpc/bits/fenvinline.h
> 
> Indeed, I misread the failures on powerpc64le-linux-gnu.  Below it is
> an updated patch with the fegetround optimization moved to an internal
> header.

> diff --git a/sysdeps/powerpc/fpu/fegetround.c b/sysdeps/powerpc/fpu/fegetround.c
> index 00b4462624..9d7762f08b 100644
> --- a/sysdeps/powerpc/fpu/fegetround.c
> +++ b/sysdeps/powerpc/fpu/fegetround.c
> @@ -21,10 +21,8 @@
>   int
>   (__fegetround) (void)
>   {
> -  return __fegetround();
> +  return __fegetround_inline ();
>   }
> -#undef fegetround
> -#undef __fegetround
>   libm_hidden_def (__fegetround)
>   weak_alias (__fegetround, fegetround)
>   libm_hidden_weak (fegetround)
> diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h
> index e888c6621c..09dbd3e2df 100644
> --- a/sysdeps/powerpc/fpu/fenv_libc.h
> +++ b/sysdeps/powerpc/fpu/fenv_libc.h
> @@ -68,6 +68,14 @@ extern const fenv_t *__fe_mask_env (void) attribute_hidden;
>       __fr;								\
>     })
>   
> +#define __fe_mffsl()							\
> +  ({register fenv_union_t __fr;						\
> +    __asm__ __volatile__ (						\
> +      ".machine push; .machine \"power9\"; mffsl %0; .machine pop"	\
> +      : "=f" (__fr.fenv));						\
> +    __fr.l & 0x3;							\
> +  })
> + >   #define __fe_mffscrn(rn)						\
>     ({register fenv_union_t __fr;						\
>       if (__builtin_constant_p (rn))					\
> @@ -144,6 +152,20 @@ typedef union
>     unsigned long long l;
>   } fenv_union_t;
>   
> +static inline int
> +__fegetround_inline (void)
> +{
> +#ifdef _ARCH_PWR9
> +  return __fe_mffsl ();
> +#else
> +  if (__glibc_likely (GLRO(dl_hwcap2) & PPC_FEATURE2_ARCH_3_00))
> +    return __fe_mffsl ();
> +
Can the above be removed, and fegetenv_register() be replaced with 
fegetenv_control()?  Such should work optimally on all ppc machines. 
Otherwise, it LGTM.

I have mixed feelings about regressing these inlines before compiler 
support arrives, but I suspect these are likely not used in performance 
critical places, so I am not objecting.

> +  fenv_union_t fe;
> +  fe.fenv = fegetenv_register ();
> +  return fe.l & 0x3;
> +#endif
> +}
>   
>   static inline int
>   __fesetround_inline (int round)

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-03-27 21:43 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-09 18:32 [PATCH v2 1/3] math: Remove fenvinline.h Adhemerval Zanella
2020-03-09 18:32 ` [PATCH 2/3] x86: Remove feraiseexcept optimization Adhemerval Zanella
2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
2020-03-27 19:56     ` H.J. Lu via Libc-alpha
2020-03-09 18:32 ` [PATCH 3/3] sparc: Move __fenv_{ld,st}fsr to fenv-private.h Adhemerval Zanella
2020-03-27 19:34   ` Adhemerval Zanella via Libc-alpha
2020-03-09 20:19 ` [PATCH v2 1/3] math: Remove fenvinline.h Paul E Murphy
2020-03-10 16:51   ` Adhemerval Zanella
2020-03-27 19:33     ` Adhemerval Zanella via Libc-alpha
2020-03-27 21:43     ` Paul E Murphy via Libc-alpha
2020-03-10 11:29 ` Florian Weimer
2020-03-10 16:58   ` Adhemerval Zanella via Libc-alpha
2020-03-10 22:54   ` Joseph Myers
2020-03-16 18:09     ` Adhemerval Zanella via Libc-alpha

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).