[Bug math/17441] New: isnan() should use __builtin_isnan() in GCC

[Bug math/17441] New: isnan() should use __builtin_isnan() in GCC

[jzwinck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

            Bug ID: 17441
           Summary: isnan() should use __builtin_isnan() in GCC
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: math
          Assignee: unassigned at sourceware dot org
          Reporter: jzwinck at gmail dot com

Today glibc's math.h defines isnan() this way:

#  define isnan(x) \
     (sizeof (x) == sizeof (float)                          \
      ? __isnanf (x)                                  \
      : sizeof (x) == sizeof (double)                          \
      ? __isnan (x) : __isnanl (x))

This works and is generic, but when compiling with GCC (and perhaps other
compilers) there is a better way:

#  define isnan(x) __builtin_isnan(x)

This is because GCC's builtin function generates a ucomisd+setp instruction
pair inline (on x86), instead of a jump to __isnan().  Given that isnan() is
often called in compute-bound mathematical operations such as loops over
matrices, this seems like a worthwhile optimization wherever __builtin__isnan()
is available.

The libstdc++ shipped with GCC already has <cmath> defining isnan() as a
trivial inline function calling __builtin_isnan().  Due to this, a C++ program
generates more efficient code if it does #include <cmath> than #include
<math.h>.

Some prior discussion of this issue is here:
http://stackoverflow.com/questions/26052640/why-does-gcc-implement-isnan-more-efficiently-for-c-cmath-than-c-math-h

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[jzwinck at gmail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

John Zwinck <jzwinck at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |performance

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[joseph at codesourcery dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

--- Comment #1 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
If making glibc classification macros use those that are built-in to GCC 
(which I agree is generally a good idea; bug 15367 makes the same request 
for isinf), there are two issues to be careful about:

* Check what GCC version made the classification built-in function 
type-generic, to avoid relying on it being type-generic in older GCC.

* Check whether the GCC version properly handles signaling NaNs; if not, 
what the header does may need to be conditional on __SUPPORT_SNAN__.  
(Properly means that the classification macro should not raise exceptions 
for a signaling NaN argument.  isnan (x) is different from isunordered (x, 
x), because the former should not raise "invalid" for signaling NaNs, but 
the latter should.)  If the GCC version does not properly handling 
signaling NaNs (given -fsignaling-nans), that should of course be reported 
as a GCC bug (and on some platforms, even calling a function may raise 
spurious exceptions for signaling NaNs - GCC bug 56831 on 32-bit x86).

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[jsm28 at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

Joseph Myers <jsm28 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[jtaylor.debian at googlemail dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

Julian Taylor <jtaylor.debian at googlemail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jtaylor.debian at googlemail dot c
                   |                            |om

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

--- Comment #2 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".

The branch, master has been updated
       via  8df4e219e43a4a257d0759b54fef8c488e2f282e (commit)
      from  cb2f668d4692df8d31b2ccf9b18b86bebe2d9174 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=8df4e219e43a4a257d0759b54fef8c488e2f282e

commit 8df4e219e43a4a257d0759b54fef8c488e2f282e
Author: Wilco Dijkstra <wdijkstr@arm.com>
Date:   Fri Sep 18 16:30:43 2015 +0100

    Add inlining of the C99 math functions
isinf/isnan/signbit/isfinite/isnormal/fpclassify using GCC
    built-ins when available. Since going through the PLT is expensive for
these small functions,
    inlining results in major speedups (about 7x on Cortex-A57 for isinf). The
GCC built-ins are not
    correct if signalling NaN support is required, and thus are turned off in
that case (see GCC bug
    66462). The test-snan.c tests sNaNs and so must be explicitly built with
-fsignaling-nans.

    2015-09-18  Wilco Dijkstra  <wdijkstr@arm.com>

            [BZ #15367]
            [BZ #17441]

            * math/Makefile: Build test-snan.c with -fsignaling-nans.
            * math/math.h (fpclassify): Use __builtin_fpclassify when
            available.  (signbit): Use __builtin_signbit(f/l).
            (isfinite): Use__builtin_isfinite.  (isnormal): Use
            __builtin_isnormal.  (isnan): Use __builtin_isnan.
            (isinf): Use __builtin_isinf_sign.

-----------------------------------------------------------------------

Summary of changes:
 ChangeLog     |   12 ++++++++++++
 NEWS          |   15 ++++++++-------
 math/Makefile |    3 ++-
 math/math.h   |   42 ++++++++++++++++++++++++++++++++++--------
 4 files changed, 56 insertions(+), 16 deletions(-)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug math/17441] isnan() should use __builtin_isnan() in GCC

[wdijkstr at arm dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=17441

Wilco <wdijkstr at arm dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |wdijkstr at arm dot com
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.23

--- Comment #3 from Wilco <wdijkstr at arm dot com> ---
Fixed

-- 
You are receiving this mail because:
You are on the CC list for the bug.