[AArch64] Add inlines for signbit (v2)

> Joseph Myers wrote:
> On Mon, 27 Apr 2015, Wilco Dijkstra wrote:
> 
> > > Joseph Myers wrote:
> > > On Wed, 15 Apr 2015, Wilco Dijkstra wrote:
> > >
> > > > Is there a reason this couldn't be done by default in math.h similar to isgreater
> (unlike
> > > > __builtin_isinf et al, GCC implements signbit efficiently and correctly).
> > >
> > > It is, however, not type-generic in GCC
> > > <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=36757>, so you'd still need
> > > to have a macro expansion checking sizeof.
> >
> > Yes indeed. However the issue is how to deal with the targets that currently
> > define inlines. Would it be reasonable just to leave them and add a signbit
> > expansion for GCC >= 3.0 that directly uses the builtins?
> > (if we define __signbit(f/l) to be __builtin_signbit(f/l), things may go
> > wrong as the various mathinlines don't do an undef first).
> 
> I'd think defining __signbit to __builtin_signbit, etc., would be
> appropriate (with conditionals in the mathinline.h files to disable their
> local definitions for GCC versions where __builtin_signbit works and is at
> least as good for that architecture).  It would also cover cases within
> glibc that directly use the __signbit etc. names (although arguably those
> should move to using the type-generic macros; likewise direct uses of e.g.
> __finite, __isnan).

OK, I tried that and it works fine without having to change all the targets if 
I define __signbit at the end of math.h.

Add inlining of the __signbit(f/l) functions for all targets using the GCC 
builtin functions when available. The out-of-line implementations need undefs
to ensure the correct symbols are exported.

OK for commit?

ChangeLog: 
2015-05-22  Wilco Dijkstra  <wdijkstr@arm.com>

	* math/math.h: (__signbit): Define.  (__signbitf): Define.
        (__signbitl): Define.	
	* sysdeps/ieee754/dbl-64/s_signbit.c: Undef __signbit. 	
	* sysdeps/ieee754/flt-32/s_signbitf.c: Undef __signbitf.
	* sysdeps/ieee754/ldbl-128/s_signbitl.c: Undef __signbitl.	
	* sysdeps/ieee754/ldbl-128ibm/s_signbitl.c: Likewise. 	
	* sysdeps/ieee754/ldbl-64-128/s_signbitl.c: Likewise.
	* sysdeps/ieee754/ldbl-96/s_signbitl.c: Likewise.

---
 math/math.h                              | 7 +++++++
 sysdeps/ieee754/dbl-64/s_signbit.c       | 3 ++-
 sysdeps/ieee754/flt-32/s_signbitf.c      | 3 ++-
 sysdeps/ieee754/ldbl-128/s_signbitl.c    | 3 ++-
 sysdeps/ieee754/ldbl-128ibm/s_signbitl.c | 2 ++
 sysdeps/ieee754/ldbl-64-128/s_signbitl.c | 1 +
 sysdeps/ieee754/ldbl-96/s_signbitl.c     | 3 ++-
 7 files changed, 18 insertions(+), 4 deletions(-)

On Fri, 22 May 2015, Wilco Dijkstra wrote:

> OK, I tried that and it works fine without having to change all the targets if 
> I define __signbit at the end of math.h.

If a target's inline __signbit definitions are completely obsolete with 
this change, they should be removed as part of it.  That means at least 
removing sysdeps/tile/bits/mathinline.h, since it only uses 
__builtin_signbit* and GCC versions before 4.0 don't support Tile.  
(ColdFire bits/mathinline.h confuses things by not having a GCC version 
conditional and so potentially using the builtins with GCC versions before 
4.0 that had ColdFire support.  But since 3.4 didn't support 
__builtin_signbit, I think that can be considered a clear bug and so the 
patch should also remove sysdeps/m68k/coldfire/fpu/bits/mathinline.h as 
obsoleted by the generic changes.)

> +# if __GNUC_PREREQ (4,0)
> +#  undef __signbitl

Why the #undef?  I don't see any headers with macro definitions of 
__signbitl that need to be removed in math.h.

> Joseph wrote:
> On Fri, 22 May 2015, Wilco Dijkstra wrote:
> 
> > OK, I tried that and it works fine without having to change all the targets if
> > I define __signbit at the end of math.h.
> 
> If a target's inline __signbit definitions are completely obsolete with
> this change, they should be removed as part of it.  That means at least
> removing sysdeps/tile/bits/mathinline.h, since it only uses
> __builtin_signbit* and GCC versions before 4.0 don't support Tile.
> (ColdFire bits/mathinline.h confuses things by not having a GCC version
> conditional and so potentially using the builtins with GCC versions before
> 4.0 that had ColdFire support.  But since 3.4 didn't support
> __builtin_signbit, I think that can be considered a clear bug and so the
> patch should also remove sysdeps/m68k/coldfire/fpu/bits/mathinline.h as
> obsoleted by the generic changes.)

OK, I'll update the patch to remove those.

> > +# if __GNUC_PREREQ (4,0)
> > +#  undef __signbitl
> 
> Why the #undef?  I don't see any headers with macro definitions of
> __signbitl that need to be removed in math.h.

Unless nldbl support is dead we need it as sysdeps/ldbl-opt/nldbl-signbit.c does:

#define __signbitl __signbitl_XXX
#include "nldbl-compat.h"

nldbl-compat.h includes math/math.h, which would define __signbitl again. 
I couldn't force nldbl to build but replacing sysdeps/ieee754/ldbl-128/s_signbitl.c
with nldlb-signbit.c confirmed the #undef fixes the issue.
 
Wilco

On Fri, 22 May 2015, Wilco Dijkstra wrote:

> > Why the #undef?  I don't see any headers with macro definitions of
> > __signbitl that need to be removed in math.h.
> 
> Unless nldbl support is dead we need it as 
> sysdeps/ldbl-opt/nldbl-signbit.c does:
> 
> #define __signbitl __signbitl_XXX
> #include "nldbl-compat.h"
> 
> nldbl-compat.h includes math/math.h, which would define __signbitl 
> again. I couldn't force nldbl to build but replacing 
> sysdeps/ieee754/ldbl-128/s_signbitl.c with nldlb-signbit.c confirmed the 
> #undef fixes the issue.

Thanks - I was only looking in headers for a #define.  I think the nldbl 
code is for linking -mlong-double-64 objects with libm, although I'm not 
aware of any documentation for it.  Having a #undef in an installed header 
that's only relevant for building glibc seems unfortunate.  Maybe it would 
be cleaner to (a) convert all glibc-internal calls to __signbit* into 
calls to the signbit macro (which should make no difference to the code 
generated at all), then (b) conditionally change the definition of the 
signbit macro to use __builtin_signbit*, rather than ever defining 
__signbit* as macros.