Re: 'signbit' patch to use 'copysign' if available

[Bruno Haible]

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