[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: 'signbit' patch to use 'copysign' if available
From: |
Bruno Haible |
Subject: |
Re: 'signbit' patch to use 'copysign' if available |
Date: |
Sat, 7 Apr 2007 12:23:53 +0200 |
User-agent: |
KMail/1.5.4 |
Hi Paul,
> Here's a proposed patch to 'signbit' to have it use 'copysign' if
> available.
Globally, I think this is backwards: 'copysign' is a more general function
than 'signbit', since it takes 2 arguments. And the usual way to return
a boolean result is in an integer register of the CPU, not in a floating-
point register. Therefore I think 'signbit' is the primitive function,
and I'm going to create modules for 'copysign' that rely on 'signbit'.
The complexity translates into code size: gnulib's signbit macro, defined
in the <math.h> replacement, expands into a bit test that will always be
faster than a call to a function that calls a function that manipulates
2 floating-point numbers.
> Normally the 'signbit' implementation relies on undefined behavior, as
> it accesses the "wrong" member of a union
This is the point of a 'union'. The code is not casting pointers; it is
using 'union's for the purpose they were defined for. And the bit positions
have been validated in the autoconf test. So where do you see undefined
behaviour?
> This is safer for traditional platforms like Solaris 9 that
> have copysign but not signbit.
What do you mean by "safer"? Solaris runs on machines with IEEE floats;
on these the signbit.m4 autoconf test will determine the right bit positions.
> 2007-04-06 Paul Eggert <address@hidden>
>
> * lib/signbitd.c (gl_signbitd): Use copysign if available.
> This avoids relying on undefined behavior.
> * lib/signbitf.c (gl_signbitf): Use copysignf if available.
> * lib/signbitl.c (gl_signbitl): Use copysignl if available.
> * m4/signbit.m4 (gl_SIGNBIT): Test for copysignf, copysign,
> copysignl.
This patch has two problems:
1) It relies on functions only available in libm. The 'signbit' module
should not rely on libm: its module description has no "Link:" section,
and it relies on isnan*-nolibm to avoid needing to link with extra
libraries.
$ ./show-portability copysignf
libc macosx-10.3
libcygwin cygwin
libm cygwin
libm freebsd-5.2.1
libm freebsd-6.0
libm hpux-11.00
libm hpux-11.11
libm netbsd-3.0
libm openbsd-3.8
libm osf1-4.0d
libm osf1-5.1a
libm solaris-2.10
libroot beos
MISSING in aix-4.3.2 aix-5.1.0 irix-6.5 mingw nsk-G06 solaris-2.4
solaris-2.5.1 solaris-2.6 solaris-2.7 solaris-2.8 solaris-2.9
$ ./show-portability copysign
libc macosx-10.3
libc nsk-G06
libcygwin cygwin
libm aix-4.3.2
libm aix-5.1.0
libm aix-5.1.0
libm cygwin
libm freebsd-5.2.1
libm freebsd-6.0
libm hpux-11.00
libm hpux-11.11
libm irix-6.5
libm netbsd-3.0
libm openbsd-3.8
libm osf1-4.0d
libm osf1-5.1a
libm solaris-2.10
libm solaris-2.4
libm solaris-2.5.1
libm solaris-2.6
libm solaris-2.7
libm solaris-2.8
libm solaris-2.9
libroot beos
MISSING in mingw
$ ./show-portability copysignl
libc macosx-10.3
libm freebsd-6.0
libm osf1-4.0d
libm osf1-5.1a
libm solaris-2.10
libroot beos
MISSING in aix-4.3.2 aix-5.1.0 cygwin freebsd-5.2.1 hpux-11.00
hpux-11.11 irix-6.5 mingw netbsd-3.0 nsk-G06 openbsd-3.8 solaris-2.4
solaris-2.5.1 solaris-2.6 solaris-2.7 solaris-2.8 solaris-2.9
So, instead of checking whether the functions are declared, the autoconf
test should use an AC_TRY_LINK check that does not use extra libraries.
The C macro name should reflect this: HAVE_COPYSIGN_IN_LIBC, to be consistent
with isnan.m4 and printf-frexp.m4.
2) The copysign-based implementation should be used as second alternative:
- not as first alternative, for performance reasons,
- but before the memcmp-based alternative, because the latter does not do the
right thing for NaNs.
Bruno
Re: 'signbit' patch to use 'copysign' if available, Bruno Haible, 2007/04/10