| Message ID | alpine.DEB.2.10.1510130053350.4306@digraph.polyomino.org.uk |
|---|---|
| State | New |
| Headers | show |
On Tue, 2015-10-13 at 00:53 +0000, Joseph Myers wrote: > The versions of llrint and llrintf for older powerpc32 processors > convert the results of __rint / __rintf to long long int, resulting in > spurious exceptions from such casts in certain cases. This patch > makes glibc work around the problems with the libgcc conversions when > the compiler used to build glibc doesn't use the fctidz instruction > for them. > > Tested for powerpc. Committed. Thanks Joseph for looking at this. This look correct as is, but I wonder if we find a way to avoid both the __rint/__rintf call and the fctidz (trunc & concert) in the normal path. The __rint implementations requires a -fPIC GOT address set up just to load a single constant. This is unfortunate as the calling function (llrint/llrintf) will have just done that. Also the fctid instruction will do the llrint function in one instruction and records NAN, overflow, as special values in the result and inexact in the FPSRC and CR1 (record form). The primary reason it was not used here was that early 32-bit PPC32 processors did not implement fctid. As Joseph has gone to the trouble to provide the configure check for HAVE_PPC_FCFID it seems like shame not to take advantage of it. Tulio we should look at this again when we have time.
On Mon, 12 Oct 2015, Steven Munroe wrote: > On Tue, 2015-10-13 at 00:53 +0000, Joseph Myers wrote: > > The versions of llrint and llrintf for older powerpc32 processors > > convert the results of __rint / __rintf to long long int, resulting in > > spurious exceptions from such casts in certain cases. This patch > > makes glibc work around the problems with the libgcc conversions when > > the compiler used to build glibc doesn't use the fctidz instruction > > for them. > > > > Tested for powerpc. Committed. > > Thanks Joseph for looking at this. > > This look correct as is, but I wonder if we find a way to avoid both the > __rint/__rintf call and the fctidz (trunc & concert) in the normal path. If you configure --with-cpu=power4 then you get versions from sysdeps/powerpc/powerpc32/power4/fpu which do just that using fctid. I suppose the issues are: (a) the GCC condition for TARGET_FCFID is quite complicated and may include some cases for which use of the power4 sysdeps directories isn't correct (because of other files there - I haven't checked); (b) while we have a desired direction for building glibc to choose sysdeps directories based on how the compiler behaves (see what sysdeps/arm/preconfigure.ac does), preferring that over glibc --with-cpu=, that hasn't been implemented for powerpc, so meaning that even if the compiler was configured with a power4 or later default and so use of the power4 sysdeps directories would be safe, glibc wouldn't use them without its own --with-cpu= configure option; (c) if both implementations were in C then it would be possible for the default one to do #if HAVE_PPC_FCTIDZ # include <sysdeps/...../power4/fpu/s_llrint.c> #else /* Other implementation. */ #endif but with one in C and one in assembler it's a bit more complicated, though that could still be done with extra source files (but given (b) it's only worthwhile if there are in fact cases where those implementations can be used but not the whole power4 sysdeps directory).
diff --git a/config.h.in b/config.h.in index 7c851c9..5fd4897 100644 --- a/config.h.in +++ b/config.h.in @@ -251,4 +251,7 @@ /* PowerPC32 uses fcfid for integer to floating point conversions. */ #define HAVE_PPC_FCFID 0 +/* PowerPC32 uses fctidz for floating point to long long conversions. */ +#define HAVE_PPC_FCTIDZ 0 + #endif diff --git a/sysdeps/powerpc/powerpc32/fpu/configure b/sysdeps/powerpc/powerpc32/fpu/configure index 69a151b..98c6f30 100644 --- a/sysdeps/powerpc/powerpc32/fpu/configure +++ b/sysdeps/powerpc/powerpc32/fpu/configure @@ -27,3 +27,30 @@ if test $libc_cv_ppc_fcfid = yes; then $as_echo "#define HAVE_PPC_FCFID 1" >>confdefs.h fi + +# Test whether floating point to long long conversions use fctidz. +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for fctidz use" >&5 +$as_echo_n "checking for fctidz use... " >&6; } +if ${libc_cv_ppc_fctidz+:} false; then : + $as_echo_n "(cached) " >&6 +else + echo 'long long int foo (double x) { return (long long int) x; }' > conftest.c +libc_cv_ppc_fctidz=no +if { ac_try='${CC-cc} -S $CFLAGS conftest.c -o conftest.s 1>&5' + { { eval echo "\"\$as_me\":${as_lineno-$LINENO}: \"$ac_try\""; } >&5 + (eval $ac_try) 2>&5 + ac_status=$? + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; }; then + if grep '[ ]fctidz' conftest.s > /dev/null 2>&1; then + libc_cv_ppc_fctidz=yes + fi +fi +rm -rf conftest* +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $libc_cv_ppc_fctidz" >&5 +$as_echo "$libc_cv_ppc_fctidz" >&6; } +if test $libc_cv_ppc_fctidz = yes; then + $as_echo "#define HAVE_PPC_FCTIDZ 1" >>confdefs.h + +fi diff --git a/sysdeps/powerpc/powerpc32/fpu/configure.ac b/sysdeps/powerpc/powerpc32/fpu/configure.ac index e596e79..1899705 100644 --- a/sysdeps/powerpc/powerpc32/fpu/configure.ac +++ b/sysdeps/powerpc/powerpc32/fpu/configure.ac @@ -16,3 +16,19 @@ rm -rf conftest*]) if test $libc_cv_ppc_fcfid = yes; then AC_DEFINE([HAVE_PPC_FCFID]) fi + +# Test whether floating point to long long conversions use fctidz. +AC_CACHE_CHECK([for fctidz use], [libc_cv_ppc_fctidz], [dnl +echo 'long long int foo (double x) { return (long long int) x; }' > conftest.c +libc_cv_ppc_fctidz=no +if AC_TRY_COMMAND(${CC-cc} -S $CFLAGS conftest.c -o conftest.s 1>&AS_MESSAGE_LOG_FD); then +changequote(,)dnl + if grep '[ ]fctidz' conftest.s > /dev/null 2>&1; then + libc_cv_ppc_fctidz=yes + fi +changequote([,])dnl +fi +rm -rf conftest*]) +if test $libc_cv_ppc_fctidz = yes; then + AC_DEFINE([HAVE_PPC_FCTIDZ]) +fi diff --git a/sysdeps/powerpc/powerpc32/fpu/s_llrint.c b/sysdeps/powerpc/powerpc32/fpu/s_llrint.c index 48af9eb..bb12272 100644 --- a/sysdeps/powerpc/powerpc32/fpu/s_llrint.c +++ b/sysdeps/powerpc/powerpc32/fpu/s_llrint.c @@ -16,13 +16,42 @@ License along with the GNU C Library; if not, see <http://www.gnu.org/licenses/>. */ +#include <limits.h> #include <math.h> #include <math_ldbl_opt.h> +#include <math_private.h> +#include <stdint.h> long long int __llrint (double x) { - return (long long int) __rint (x); + double rx = __rint (x); + if (HAVE_PPC_FCTIDZ || rx != x) + return (long long int) rx; + else + { + /* Avoid incorrect exceptions from libgcc conversions (as of GCC + 5): <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59412>. */ + if (fabs (rx) < 0x1p31) + return (long long int) (long int) rx; + uint64_t i0; + EXTRACT_WORDS64 (i0, rx); + int exponent = ((i0 >> 52) & 0x7ff) - 0x3ff; + if (exponent < 63) + { + unsigned long long int mant + = (i0 & ((1ULL << 52) - 1)) | (1ULL << 52); + if (exponent < 52) + mant >>= 52 - exponent; + else + mant <<= exponent - 52; + return (long long int) ((i0 & (1ULL << 63)) != 0 ? -mant : mant); + } + else if (rx == (double) LLONG_MIN) + return LLONG_MIN; + else + return (long long int) (long int) rx << 32; + } } weak_alias (__llrint, llrint) #ifdef NO_LONG_DOUBLE diff --git a/sysdeps/powerpc/powerpc32/fpu/s_llrintf.c b/sysdeps/powerpc/powerpc32/fpu/s_llrintf.c index 205df4e..bcee020 100644 --- a/sysdeps/powerpc/powerpc32/fpu/s_llrintf.c +++ b/sysdeps/powerpc/powerpc32/fpu/s_llrintf.c @@ -17,10 +17,30 @@ <http://www.gnu.org/licenses/>. */ #include <math.h> +#include <math_private.h> +#include <stdint.h> long long int __llrintf (float x) { - return (long long int) __rintf (x); + float rx = __rintf (x); + if (HAVE_PPC_FCTIDZ || rx != x) + return (long long int) rx; + else + { + float arx = fabsf (rx); + /* Avoid incorrect exceptions from libgcc conversions (as of GCC + 5): <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59412>. */ + if (arx < 0x1p31f) + return (long long int) (long int) rx; + else if (!(arx < 0x1p55f)) + return (long long int) (long int) (rx * 0x1p-32f) << 32; + uint32_t i0; + GET_FLOAT_WORD (i0, rx); + int exponent = ((i0 >> 23) & 0xff) - 0x7f; + unsigned long long int mant = (i0 & 0x7fffff) | 0x800000; + mant <<= exponent - 23; + return (long long int) ((i0 & 0x80000000) != 0 ? -mant : mant); + } } weak_alias (__llrintf, llrintf)