| Message ID | alpine.DEB.2.10.1412301944240.17381@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
On 12/30/2014 02:45 PM, Joseph Myers wrote: > The natural fix for some linknamespace test failures, where C90 libm > functions call C99 <fenv.h> functions, is to make fe* into weak > aliases for __fe* and call __fe* from within libm as needed. > > To do this, the __fe* names need to be available for that purpose - > that is, they must not be used for something other than aliases of > fe*. On powerpc, however, __fegetround is an inline function in > fenv_libc.h, with no corresponding fegetround inline function; > fegetround has an equivalent macro expansion in bits/fenvinline.h, but > that is disabled if __NO_MATH_INLINES (which is defined for building > libm). > > I see no need for that disabling; it's not even clear that > __NO_MATH_INLINES should affect <fenv.h>, and the results of > fegetround are completely defined so there is no semantic effect of > that disabling at all outside glibc. The x86 inline feraiseexcept is > conditioned on __USE_EXTERN_INLINES not __NO_MATH_INLINES (but that's > an inline function rather than a macro). > > This patch removes the __NO_MATH_INLINES conditional on that > fegetround macro, so resulting in it being expanded inline inside > glibc. In turn, this means that direct calls to __fegetround from C99 > functions in ldbl-128ibm can be changed to calls to fegetround, so > that nofpu fenv_libc.h files don't need to define __fegetround at all > and, by changing ldbl-128ibm files to use <fenv.h> not <fenv_libc.h>, > non-e500 nofpu no longer needs an fenv_libc.h file. > > The other macros in fenvinline.h are left conditional on > __NO_MATH_INLINES, although since the only case where this should make > a difference is one involving undefined behavior (if the argument to > the function is not a valid exception macro). > > The out-of-line definition for fegetround uses __fegetround (the > inline function removed by this patch). So this continues to work, > the fenvinline.h header is made to define __fegetround, and then to > define fegetround to call __fegetround. > > Tested for powerpc32 (hard float) that installed stripped shared > libraries are unchanged by this patch; also tested that powerpc-nofpu > build still works. (This patch does not itself fix any bugs; it > simply cleans things up in preparation for separate bug fixes.) > > 2014-12-30 Joseph Myers <joseph@codesourcery.com> > > * sysdeps/powerpc/bits/fenvinline.h (fegetround): Rename macro to > __fegetround and redefine to call __fegetround. Remove condition > on [!__NO_MATH_INLINES]. > * sysdeps/powerpc/fpu/fenv_libc.h (__fegetround): Remove inline > function. > * sysdeps/powerpc/nofpu/fenv_libc.h: Remove file. > * sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h (__fegetround): > Remove macro. > * sysdeps/ieee754/ldbl-128ibm/s_llrintl.c: Include <fenv.h> > instead of <fenv_libc.h>. > (__llrintl): Call fegetround instead of __fegetround. > * sysdeps/ieee754/ldbl-128ibm/s_llroundl.c: Include <fenv.h> > instead of <fenv_libc.h>. > * sysdeps/ieee754/ldbl-128ibm/s_lrintl.c: Likewise. > (__lrintl): Call fegetround instead of __fegetround. > * sysdeps/ieee754/ldbl-128ibm/s_lroundl.c: Include <fenv.h> > instead of <fenv_libc.h>. > * sysdeps/ieee754/ldbl-128ibm/s_rintl.c: Likewise. > (__rintl): Call fegetround instead of __fegetround. Looks good to me. Is there any reason you're not using [PATCH] in the prefix? Is this a WIP or RFC? > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c b/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c > index 86996bf..3d9dee1 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c > @@ -18,7 +18,7 @@ > <http://www.gnu.org/licenses/>. */ > > #include <math.h> > -#include <fenv_libc.h> > +#include <fenv.h> > #include <math_ldbl_opt.h> > #include <float.h> > #include <ieee754.h> > @@ -43,7 +43,7 @@ __llrintl (long double x) > #endif > ) > { > - save_round = __fegetround (); > + save_round = fegetround (); > > if (__glibc_unlikely ((xh == -(double) (-__LONG_LONG_MAX__ - 1)))) > { > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c b/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c > index dd84074..98e4253 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c > @@ -18,7 +18,7 @@ > <http://www.gnu.org/licenses/>. */ > > #include <math.h> > -#include <fenv_libc.h> > +#include <fenv.h> > #include <math_ldbl_opt.h> > #include <float.h> > #include <ieee754.h> > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c b/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c > index 2b47c52..895a066 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c > @@ -18,7 +18,7 @@ > <http://www.gnu.org/licenses/>. */ > > #include <math.h> > -#include <fenv_libc.h> > +#include <fenv.h> > #include <math_ldbl_opt.h> > #include <float.h> > #include <ieee754.h> > @@ -49,7 +49,7 @@ __lrintl (long double x) > #endif > ) > { > - save_round = __fegetround (); > + save_round = fegetround (); > > #if __LONG_MAX__ == 2147483647 > long long llhi = (long long) xh; > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c b/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c > index 27b72e2..6c3a314 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c > @@ -18,7 +18,7 @@ > <http://www.gnu.org/licenses/>. */ > > #include <math.h> > -#include <fenv_libc.h> > +#include <fenv.h> > #include <math_ldbl_opt.h> > #include <float.h> > #include <ieee754.h> > diff --git a/sysdeps/ieee754/ldbl-128ibm/s_rintl.c b/sysdeps/ieee754/ldbl-128ibm/s_rintl.c > index 20b75dd..f39e6fd 100644 > --- a/sysdeps/ieee754/ldbl-128ibm/s_rintl.c > +++ b/sysdeps/ieee754/ldbl-128ibm/s_rintl.c > @@ -21,7 +21,7 @@ > when it's coded in C. */ > > #include <math.h> > -#include <fenv_libc.h> > +#include <fenv.h> > #include <math_ldbl_opt.h> > #include <float.h> > #include <ieee754.h> > @@ -40,7 +40,7 @@ __rintl (long double x) > __builtin_inf ()), 1)) > { > double orig_xh; > - int save_round = __fegetround (); > + int save_round = fegetround (); > > /* Long double arithmetic, including the canonicalisation below, > only works in round-to-nearest mode. */ > diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h > index 00336f3..e914928 100644 > --- a/sysdeps/powerpc/bits/fenvinline.h > +++ b/sysdeps/powerpc/bits/fenvinline.h > @@ -16,23 +16,24 @@ > License along with the GNU C Library; if not, see > <http://www.gnu.org/licenses/>. */ > > -#if (defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__ \ > - && !defined __NO_MATH_INLINES) > +#if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__ > > /* Inline definition for fegetround. */ > -# define fegetround() \ > +# define __fegetround() \ > (__extension__ ({ int __fegetround_result; \ > __asm__ __volatile__ \ > ("mcrfs 7,7 ; mfcr %0" \ > : "=r"(__fegetround_result) : : "cr7"); \ > __fegetround_result & 3; })) > +# define fegetround() __fegetround () > > +# ifndef __NO_MATH_INLINES > /* 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'. */ > > /* Inline definition for feraiseexcept. */ > -# define feraiseexcept(__excepts) \ > +# define feraiseexcept(__excepts) \ > ((__builtin_constant_p (__excepts) \ > && ((__excepts) & ((__excepts)-1)) == 0 \ > && (__excepts) != FE_INVALID) \ > @@ -45,7 +46,7 @@ > : (feraiseexcept) (__excepts)) > > /* Inline definition for feclearexcept. */ > -# define feclearexcept(__excepts) \ > +# define feclearexcept(__excepts) \ > ((__builtin_constant_p (__excepts) \ > && ((__excepts) & ((__excepts)-1)) == 0 \ > && (__excepts) != FE_INVALID) \ > @@ -57,4 +58,6 @@ > : 0) \ > : (feclearexcept) (__excepts)) > > +# endif /* !__NO_MATH_INLINES. */ > + > #endif /* __GNUC__ && !_SOFT_FLOAT && !__NO_FPRS__ */ > diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h > index c1df5ce..125745e 100644 > --- a/sysdeps/powerpc/fpu/fenv_libc.h > +++ b/sysdeps/powerpc/fpu/fenv_libc.h > @@ -76,15 +76,6 @@ typedef union > > > static inline int > -__fegetround (void) > -{ > - int result; > - asm volatile ("mcrfs 7,7\n\t" > - "mfcr %0" : "=r"(result) : : "cr7"); > - return result & 3; > -} > - > -static inline int > __fesetround (int round) > { > if ((unsigned int) round < 2) > diff --git a/sysdeps/powerpc/nofpu/fenv_libc.h b/sysdeps/powerpc/nofpu/fenv_libc.h > deleted file mode 100644 > index dce1524..0000000 > --- a/sysdeps/powerpc/nofpu/fenv_libc.h > +++ /dev/null > @@ -1,31 +0,0 @@ > -/* Internal libc stuff for floating point environment routines. > - Copyright (C) 2007-2014 Free Software Foundation, Inc. > - This file is part of the GNU C Library. > - > - The GNU C Library is free software; you can redistribute it and/or > - modify it under the terms of the GNU Lesser General Public > - License as published by the Free Software Foundation; either > - version 2.1 of the License, or (at your option) any later version. > - > - The GNU C Library is distributed in the hope that it will be useful, > - but WITHOUT ANY WARRANTY; without even the implied warranty of > - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > - Lesser General Public License for more details. > - > - You should have received a copy of the GNU Lesser General Public > - License along with the GNU C Library. If not, see > - <http://www.gnu.org/licenses/>. */ > - > -#ifndef _FENV_LIBC_H > -#define _FENV_LIBC_H 1 > - > -/* fenv_libc.h is used in libm implementations of ldbl-128ibm. So we > - need this version in the soft-fp to at minimum include fenv.h to > - get the fegetround definition. */ > - > -#include <fenv.h> > - > -/* ldbl-128ibm code uses __fegetround. */ > -#define __fegetround() fegetround () > - > -#endif /* fenv_libc.h */ > diff --git a/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h b/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h > index d8225e7..07eb675 100644 > --- a/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h > +++ b/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h > @@ -21,9 +21,6 @@ > > #include <fenv.h> > > -/* ldbl-128ibm code uses __fegetround. */ > -#define __fegetround() fegetround () > - > int __feraiseexcept_spe (int); > libm_hidden_proto (__feraiseexcept_spe) > >
On Tue, 30 Dec 2014, Carlos O'Donell wrote: > Is there any reason you're not using [PATCH] in the prefix? I don't find [PATCH] markers useful (indeed, I find them to have negative utility, just serving to hide and distract from substantive information); I think patches can be assumed as the default. > Is this a WIP or RFC? It's a patch proposed for inclusion that I hope Adhemerval might review / comment on.
On 12/30/2014 03:01 PM, Joseph Myers wrote: > On Tue, 30 Dec 2014, Carlos O'Donell wrote: > >> Is there any reason you're not using [PATCH] in the prefix? > > I don't find [PATCH] markers useful (indeed, I find them to have negative > utility, just serving to hide and distract from substantive information); > I think patches can be assumed as the default. Thanks for explaining. >> Is this a WIP or RFC? > > It's a patch proposed for inclusion that I hope Adhemerval might review / > comment on. Thanks again. Cheers, Carlos.
On 30-12-2014 18:01, Joseph Myers wrote: > On Tue, 30 Dec 2014, Carlos O'Donell wrote: > >> Is there any reason you're not using [PATCH] in the prefix? > I don't find [PATCH] markers useful (indeed, I find them to have negative > utility, just serving to hide and distract from substantive information); > I think patches can be assumed as the default. > >> Is this a WIP or RFC? > It's a patch proposed for inclusion that I hope Adhemerval might review / > comment on. > I will review/check it later today, thanks.
On 30-12-2014 17:45, Joseph Myers wrote: > The natural fix for some linknamespace test failures, where C90 libm > functions call C99 <fenv.h> functions, is to make fe* into weak > aliases for __fe* and call __fe* from within libm as needed. > > To do this, the __fe* names need to be available for that purpose - > that is, they must not be used for something other than aliases of > fe*. On powerpc, however, __fegetround is an inline function in > fenv_libc.h, with no corresponding fegetround inline function; > fegetround has an equivalent macro expansion in bits/fenvinline.h, but > that is disabled if __NO_MATH_INLINES (which is defined for building > libm). > > I see no need for that disabling; it's not even clear that > __NO_MATH_INLINES should affect <fenv.h>, and the results of > fegetround are completely defined so there is no semantic effect of > that disabling at all outside glibc. The x86 inline feraiseexcept is > conditioned on __USE_EXTERN_INLINES not __NO_MATH_INLINES (but that's > an inline function rather than a macro). > > This patch removes the __NO_MATH_INLINES conditional on that > fegetround macro, so resulting in it being expanded inline inside > glibc. In turn, this means that direct calls to __fegetround from C99 > functions in ldbl-128ibm can be changed to calls to fegetround, so > that nofpu fenv_libc.h files don't need to define __fegetround at all > and, by changing ldbl-128ibm files to use <fenv.h> not <fenv_libc.h>, > non-e500 nofpu no longer needs an fenv_libc.h file. > > The other macros in fenvinline.h are left conditional on > __NO_MATH_INLINES, although since the only case where this should make > a difference is one involving undefined behavior (if the argument to > the function is not a valid exception macro). > > The out-of-line definition for fegetround uses __fegetround (the > inline function removed by this patch). So this continues to work, > the fenvinline.h header is made to define __fegetround, and then to > define fegetround to call __fegetround. > > Tested for powerpc32 (hard float) that installed stripped shared > libraries are unchanged by this patch; also tested that powerpc-nofpu > build still works. (This patch does not itself fix any bugs; it > simply cleans things up in preparation for separate bug fixes.) LGTM, I checked on powerpc64 and powerpc64le and saw no regressions. Thanks!
diff --git a/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c b/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c index 86996bf..3d9dee1 100644 --- a/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c +++ b/sysdeps/ieee754/ldbl-128ibm/s_llrintl.c @@ -18,7 +18,7 @@ <http://www.gnu.org/licenses/>. */ #include <math.h> -#include <fenv_libc.h> +#include <fenv.h> #include <math_ldbl_opt.h> #include <float.h> #include <ieee754.h> @@ -43,7 +43,7 @@ __llrintl (long double x) #endif ) { - save_round = __fegetround (); + save_round = fegetround (); if (__glibc_unlikely ((xh == -(double) (-__LONG_LONG_MAX__ - 1)))) { diff --git a/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c b/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c index dd84074..98e4253 100644 --- a/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c +++ b/sysdeps/ieee754/ldbl-128ibm/s_llroundl.c @@ -18,7 +18,7 @@ <http://www.gnu.org/licenses/>. */ #include <math.h> -#include <fenv_libc.h> +#include <fenv.h> #include <math_ldbl_opt.h> #include <float.h> #include <ieee754.h> diff --git a/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c b/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c index 2b47c52..895a066 100644 --- a/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c +++ b/sysdeps/ieee754/ldbl-128ibm/s_lrintl.c @@ -18,7 +18,7 @@ <http://www.gnu.org/licenses/>. */ #include <math.h> -#include <fenv_libc.h> +#include <fenv.h> #include <math_ldbl_opt.h> #include <float.h> #include <ieee754.h> @@ -49,7 +49,7 @@ __lrintl (long double x) #endif ) { - save_round = __fegetround (); + save_round = fegetround (); #if __LONG_MAX__ == 2147483647 long long llhi = (long long) xh; diff --git a/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c b/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c index 27b72e2..6c3a314 100644 --- a/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c +++ b/sysdeps/ieee754/ldbl-128ibm/s_lroundl.c @@ -18,7 +18,7 @@ <http://www.gnu.org/licenses/>. */ #include <math.h> -#include <fenv_libc.h> +#include <fenv.h> #include <math_ldbl_opt.h> #include <float.h> #include <ieee754.h> diff --git a/sysdeps/ieee754/ldbl-128ibm/s_rintl.c b/sysdeps/ieee754/ldbl-128ibm/s_rintl.c index 20b75dd..f39e6fd 100644 --- a/sysdeps/ieee754/ldbl-128ibm/s_rintl.c +++ b/sysdeps/ieee754/ldbl-128ibm/s_rintl.c @@ -21,7 +21,7 @@ when it's coded in C. */ #include <math.h> -#include <fenv_libc.h> +#include <fenv.h> #include <math_ldbl_opt.h> #include <float.h> #include <ieee754.h> @@ -40,7 +40,7 @@ __rintl (long double x) __builtin_inf ()), 1)) { double orig_xh; - int save_round = __fegetround (); + int save_round = fegetround (); /* Long double arithmetic, including the canonicalisation below, only works in round-to-nearest mode. */ diff --git a/sysdeps/powerpc/bits/fenvinline.h b/sysdeps/powerpc/bits/fenvinline.h index 00336f3..e914928 100644 --- a/sysdeps/powerpc/bits/fenvinline.h +++ b/sysdeps/powerpc/bits/fenvinline.h @@ -16,23 +16,24 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ -#if (defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__ \ - && !defined __NO_MATH_INLINES) +#if defined __GNUC__ && !defined _SOFT_FLOAT && !defined __NO_FPRS__ /* Inline definition for fegetround. */ -# define fegetround() \ +# define __fegetround() \ (__extension__ ({ int __fegetround_result; \ __asm__ __volatile__ \ ("mcrfs 7,7 ; mfcr %0" \ : "=r"(__fegetround_result) : : "cr7"); \ __fegetround_result & 3; })) +# define fegetround() __fegetround () +# ifndef __NO_MATH_INLINES /* 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'. */ /* Inline definition for feraiseexcept. */ -# define feraiseexcept(__excepts) \ +# define feraiseexcept(__excepts) \ ((__builtin_constant_p (__excepts) \ && ((__excepts) & ((__excepts)-1)) == 0 \ && (__excepts) != FE_INVALID) \ @@ -45,7 +46,7 @@ : (feraiseexcept) (__excepts)) /* Inline definition for feclearexcept. */ -# define feclearexcept(__excepts) \ +# define feclearexcept(__excepts) \ ((__builtin_constant_p (__excepts) \ && ((__excepts) & ((__excepts)-1)) == 0 \ && (__excepts) != FE_INVALID) \ @@ -57,4 +58,6 @@ : 0) \ : (feclearexcept) (__excepts)) +# endif /* !__NO_MATH_INLINES. */ + #endif /* __GNUC__ && !_SOFT_FLOAT && !__NO_FPRS__ */ diff --git a/sysdeps/powerpc/fpu/fenv_libc.h b/sysdeps/powerpc/fpu/fenv_libc.h index c1df5ce..125745e 100644 --- a/sysdeps/powerpc/fpu/fenv_libc.h +++ b/sysdeps/powerpc/fpu/fenv_libc.h @@ -76,15 +76,6 @@ typedef union static inline int -__fegetround (void) -{ - int result; - asm volatile ("mcrfs 7,7\n\t" - "mfcr %0" : "=r"(result) : : "cr7"); - return result & 3; -} - -static inline int __fesetround (int round) { if ((unsigned int) round < 2) diff --git a/sysdeps/powerpc/nofpu/fenv_libc.h b/sysdeps/powerpc/nofpu/fenv_libc.h deleted file mode 100644 index dce1524..0000000 --- a/sysdeps/powerpc/nofpu/fenv_libc.h +++ /dev/null @@ -1,31 +0,0 @@ -/* Internal libc stuff for floating point environment routines. - Copyright (C) 2007-2014 Free Software Foundation, Inc. - This file is part of the GNU C Library. - - The GNU C Library is free software; you can redistribute it and/or - modify it under the terms of the GNU Lesser General Public - License as published by the Free Software Foundation; either - version 2.1 of the License, or (at your option) any later version. - - The GNU C Library is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU - Lesser General Public License for more details. - - You should have received a copy of the GNU Lesser General Public - License along with the GNU C Library. If not, see - <http://www.gnu.org/licenses/>. */ - -#ifndef _FENV_LIBC_H -#define _FENV_LIBC_H 1 - -/* fenv_libc.h is used in libm implementations of ldbl-128ibm. So we - need this version in the soft-fp to at minimum include fenv.h to - get the fegetround definition. */ - -#include <fenv.h> - -/* ldbl-128ibm code uses __fegetround. */ -#define __fegetround() fegetround () - -#endif /* fenv_libc.h */ diff --git a/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h b/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h index d8225e7..07eb675 100644 --- a/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h +++ b/sysdeps/powerpc/powerpc32/e500/nofpu/fenv_libc.h @@ -21,9 +21,6 @@ #include <fenv.h> -/* ldbl-128ibm code uses __fegetround. */ -#define __fegetround() fegetround () - int __feraiseexcept_spe (int); libm_hidden_proto (__feraiseexcept_spe)