[Bug math/15367] New: Let gcc use __builtin_isinf

[Bug math/15367] New: Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

             Bug #: 15367
           Summary: Let gcc use __builtin_isinf
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: normal
          Priority: P2
         Component: math
        AssignedTo: unassigned@sourceware.org
        ReportedBy: marc.glisse@normalesup.org
    Classification: Unclassified


Hello,

gcc has had __builtin_isinf for a while now, but not __builtin___isinf.
However, math.h defines a macro that replaces isinf with __isinf and thus
prevents gcc from inlining its implementation. I thus get better code with
(isinf)(d) than isinf(d), because it inhibits the macro expansion.

Could the macro be changed, or do we have to add __builtin___isinf to gcc?

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[schwab@linux-m68k.org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> 2013-04-15 09:52:36 UTC ---
The standard only defines isinf as a macro, so it cannot refer to an isinf
function.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #2 from Marc Glisse <marc.glisse at normalesup dot org> 2013-04-15 10:01:45 UTC ---
The way isgreater (and several others) is handled seems like it could work for
isinf as well (except that the minimal gcc version may be 4.3 instead of 2.97).

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[joseph at codesourcery dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #3 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2013-04-19 20:28:11 UTC ---
glibc's isinf corresponds closer to __builtin_isinf_sign than 
__builtin_isinf, but it seems to document even stricter requirements on 
the return value (i.e. that for glibc it will be +1 or -1 as applicable 
for infinite argument, rather than unspecified positive or negative values 
for __builtin_isinf_sign).

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #4 from Marc Glisse <marc.glisse at normalesup dot org> 2013-04-19 20:41:12 UTC ---
Ah, indeed... This only leaves isnan, isfinite, signbit and isnormal? (maybe
fpclassify as well, but it seems more complicated for less gain)

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[jtaylor.debian at googlemail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

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

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

--- Comment #5 from Julian Taylor <jtaylor.debian at googlemail dot com> 2013-05-08 18:20:54 UTC ---
This would be very useful.
Lots of software has to add explicit __builtin_isnan/isinf/isfinite to their
headers or undefine the macros to get decent performance with glibc.
Checking for isnan/isinf/isfinite in tight loops is very common in numerical
applications.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[jtaylor.debian at googlemail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #6 from Julian Taylor <jtaylor.debian at googlemail dot com> 2013-05-08 18:48:28 UTC ---
judging from this mail it seems builtin_isinf_sign was explicitly added so it
can be used in C libraries:
http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01067.html

the patch documents +1 and -1 as return values, just as glibc does.
The source is still the same in gcc 4.8.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #7 from Marc Glisse <marc.glisse at normalesup dot org> 2013-05-08 19:02:05 UTC ---
(In reply to comment #6)
> judging from this mail it seems builtin_isinf_sign was explicitly added so it
> can be used in C libraries:
> http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01067.html
> 
> the patch documents +1 and -1 as return values, just as glibc does.
> The source is still the same in gcc 4.8.

http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html#index-g_t_005f_005fbuiltin_005fisinf_005fsign-3391

"Similar to isinf, except the return value is negative for an argument of
-Inf."

That doesn't seem to be restricted to +1 and -1. You may want to ask gcc for a
stricter doc.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[jtaylor.debian at googlemail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #8 from Julian Taylor <jtaylor.debian at googlemail dot com> 2013-05-08 19:20:38 UTC ---
you could do:
__builtin_isinf_sign((x)) % 2
to guarantee a unit result. It optimizes the modulo away with gcc-4.7 as _sign
returns 1 or -1.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #9 from Marc Glisse <marc.glisse at normalesup dot org> 2013-05-08 19:41:52 UTC ---
(In reply to comment #8)
> you could do:
> __builtin_isinf_sign((x)) % 2
> to guarantee a unit result.

That's even worse. Either you trust __builtin_isinf_sign to return -1/0/+1, or
you explicitly test:
int s=__builtin_isinf_sign(x);
if(s<0) return -1;
else return s>0;

If the builtin were to return -2, the modulo would be nonsense.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[jtaylor.debian at googlemail dot com]

http://sourceware.org/bugzilla/show_bug.cgi?id=15367

--- Comment #10 from Julian Taylor <jtaylor.debian at googlemail dot com> 2013-05-08 21:06:57 UTC ---
yes of course, how stupid of me.
But even with the explicit check it is still a good 25% faster than the
function call version for arrays with no infs, and full of infs on my amd64
phenom X4 on eglibc 2.17.

-- 
Configure bugmail: http://sourceware.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.

[Bug math/15367] Let gcc use __builtin_isinf

[marc.glisse at normalesup dot org]

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

--- Comment #11 from Marc Glisse <marc.glisse at normalesup dot org> ---
The gcc documentation of __builtin_isinf_sign has just been updated, so it can
now be used to implement isinf in glibc.
http://gcc.gnu.org/viewcvs/gcc?view=revision&revision=202021
(the code was always fine, this is only important as a promise not to change
the behavior in the future)

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

[Bug math/15367] Let gcc use __builtin_isinf

[jsm28 at gcc dot gnu.org]

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

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/15367] Let gcc use __builtin_isinf

[fweimer at redhat dot com]

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

Florian Weimer <fweimer at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|                            |security-

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

[Bug math/15367] Let gcc use __builtin_isinf

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 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.