[Bug math/15384] New: One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[Bug math/15384] New: One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[marc.glisse at normalesup dot org]

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

             Bug #: 15384
           Summary: One constant fewer in
                    ieee754/dbl-64/wordsize-64/s_finite.c
           Product: glibc
           Version: unspecified
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: math
        AssignedTo: unassigned@sourceware.org
        ReportedBy: marc.glisse@normalesup.org
    Classification: Unclassified


Hello,

this is very minor, and I don't even know what platforms use this file, but in
ieee754/dbl-64/wordsize-64/s_finite.c:

int
__finite(double x)
{
  int64_t lx;
  EXTRACT_WORDS64(lx,x);
  return
(int)((uint64_t)((lx&INT64_C(0x7fffffffffffffff))-INT64_C(0x7ff0000000000000))>>63);
}

The first constant 0x7fffffffffffffff is unnecessary, it would work just as
well with 0x7ff0000000000000, so there is only one constant loaded. The same
applies to the 32 bit version, but for platforms like x86 that embed 32 bit
constants in the instruction it probably wouldn't matter.

-- 
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.

Re: [Bug math/15384] New: One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[Ondřej Bílka]

On Sun, Apr 21, 2013 at 10:06:00AM +0000, marc.glisse at normalesup dot org wrote:
> http://sourceware.org/bugzilla/show_bug.cgi?id=15384
> 
>              Bug #: 15384
>            Summary: One constant fewer in
>                     ieee754/dbl-64/wordsize-64/s_finite.c
>            Product: glibc
>            Version: unspecified
>             Status: NEW
>           Severity: enhancement
>           Priority: P2
>          Component: math
>         AssignedTo: unassigned@sourceware.org
>         ReportedBy: marc.glisse@normalesup.org
>     Classification: Unclassified
> 
> 
> Hello,
> 
> this is very minor, and I don't even know what platforms use this file, but in
> ieee754/dbl-64/wordsize-64/s_finite.c:
> 
> int
> __finite(double x)
> {
>   int64_t lx;
>   EXTRACT_WORDS64(lx,x);
>   return
> (int)((uint64_t)((lx&INT64_C(0x7fffffffffffffff))-INT64_C(0x7ff0000000000000))>>63);
> }
>
It could be simplified more. Constants are  not necessary here. I will
post patch to libc-alpha. 
> The first constant 0x7fffffffffffffff is unnecessary, it would work just as
> well with 0x7ff0000000000000, so there is only one constant loaded. The same
> applies to the 32 bit version, but for platforms like x86 that embed 32 bit
> constants in the instruction it probably wouldn't matter.
> 
> -- 
> 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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[neleai at seznam dot cz]

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

--- Comment #1 from Ondrej Bilka <neleai at seznam dot cz> 2013-04-21 10:54:14 UTC ---
On Sun, Apr 21, 2013 at 10:06:00AM +0000, marc.glisse at normalesup dot org
wrote:
> http://sourceware.org/bugzilla/show_bug.cgi?id=15384
> 
>              Bug #: 15384
>            Summary: One constant fewer in
>                     ieee754/dbl-64/wordsize-64/s_finite.c
>            Product: glibc
>            Version: unspecified
>             Status: NEW
>           Severity: enhancement
>           Priority: P2
>          Component: math
>         AssignedTo: unassigned@sourceware.org
>         ReportedBy: marc.glisse@normalesup.org
>     Classification: Unclassified
> 
> 
> Hello,
> 
> this is very minor, and I don't even know what platforms use this file, but in
> ieee754/dbl-64/wordsize-64/s_finite.c:
> 
> int
> __finite(double x)
> {
>   int64_t lx;
>   EXTRACT_WORDS64(lx,x);
>   return
> (int)((uint64_t)((lx&INT64_C(0x7fffffffffffffff))-INT64_C(0x7ff0000000000000))>>63);
> }
>
It could be simplified more. Constants are  not necessary here. I will
post patch to libc-alpha. 
> The first constant 0x7fffffffffffffff is unnecessary, it would work just as
> well with 0x7ff0000000000000, so there is only one constant loaded. The same
> applies to the 32 bit version, but for platforms like x86 that embed 32 bit
> constants in the instruction it probably wouldn't matter.
> 
> -- 
> 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.

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[ondra at iuuk dot mff.cuni.cz]

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

OndrejBilka <ondra at iuuk dot mff.cuni.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at sourceware    |ondra at iuuk dot
                   |dot org                     |mff.cuni.cz

--- Comment #2 from OndrejBilka <ondra at iuuk dot mff.cuni.cz> 2013-04-21 11:03:03 UTC ---
Created attachment 6991
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6991
benchmark

Benchmark for new version

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[ondra at iuuk dot mff.cuni.cz]

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

OndrejBilka <ondra at iuuk dot mff.cuni.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6991|0                           |1
        is obsolete|                            |

--- Comment #3 from OndrejBilka <ondra at iuuk dot mff.cuni.cz> 2013-04-21 14:39:14 UTC ---
Created attachment 6992
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6992
new benchmark

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[ondra at iuuk dot mff.cuni.cz]

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

OndrejBilka <ondra at iuuk dot mff.cuni.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6992|0                           |1
        is obsolete|                            |

--- Comment #4 from OndrejBilka <ondra at iuuk dot mff.cuni.cz> 2013-04-21 14:46:18 UTC ---
Created attachment 6993
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6993
new benchmark

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[marc.glisse at normalesup dot org]

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

--- Comment #5 from Marc Glisse <marc.glisse at normalesup dot org> 2013-04-21 15:44:26 UTC ---
Created attachment 6994
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6994
Other bench

Hello,

this is what I've been testing with, compiled with (possible zsh-ism):

for i in "" {0..7}; do echo finite$i; gcc test.c -O3 -Wall -W
-fno-builtin-finite -Dfinite=finite$i; repeat 11 time ./a.out; done

and your new code does not appear to beat the old one in speed (for size, the
non-null version looks great). Interestingly, __builtin_finite wins this test,
although you can feed it a denormal if you want to beat it.

The more I try, the less convinced I get we should change anything :-(

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[marc.glisse at normalesup dot org]

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

--- Comment #6 from Marc Glisse <marc.glisse at normalesup dot org> 2013-04-21 15:59:00 UTC ---
Are you sure about the constants 127 and 7 in your code?

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[ondra at iuuk dot mff.cuni.cz]

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

--- Comment #7 from OndrejBilka <ondra at iuuk dot mff.cuni.cz> 2013-04-21 17:27:51 UTC ---
Created attachment 6995
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6995
benchmark for inlines

I updated your benchmark. Use

for i in `seq 0 7`; do echo finite$i;  gcc test.c -O3 -Wall -W
-fno-builtin-finite -Dfinite=finite$i; for j in `seq 1 8`; do /usr/bin/time -f
"%U" ./a.out; done; done

which works in bash.

> and your new code does not appear to beat the old one in speed (for size, the
> non-null version looks great). Interestingly, __builtin_finite wins this test,
> although you can feed it a denormal if you want to beat it.
This depends on platform and benchmark.

Performance of noninlined function does not make lot of sense because gcc
inlines them all. This was partly reason I optimize for size.

As I bet that 99% of use cases are something like
if (isfinite(x)) stuff() else error();
I added benchmark for testing a branch. Then my modified versions seem to win.
on nehalem

Last implementation correctly computed 1 for infinite values and 0 otherwise.

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[marc.glisse at normalesup dot org]

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

--- Comment #8 from Marc Glisse <marc.glisse at normalesup dot org> 2013-04-21 18:06:44 UTC ---
(In reply to comment #7)
> Performance of noninlined function does not make lot of sense because gcc
> inlines them all.

I thought for glibc it did make sense, since the glibc functions won't be
inlined. Now optimizing the gcc builtins is a different story, those are always
inlined (and there you also want to check how well the expansion interacts with
the vectorizer).

> This was partly reason I optimize for size.

That makes sense.

> As I bet that 99% of use cases are something like
> if (isfinite(x)) stuff() else error();
> I added benchmark for testing a branch. Then my modified versions seem to win.
> on nehalem
> 
> Last implementation correctly computed 1 for infinite values and 0 otherwise.

finite7 from your "benchmark for inlines" is giving me a different answer for
1.0 and 2.0. I would understand it better with 0x7ff instead of 127.

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[ondra at iuuk dot mff.cuni.cz]

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

OndrejBilka <ondra at iuuk dot mff.cuni.cz> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #6995|0                           |1
        is obsolete|                            |

--- Comment #9 from OndrejBilka <ondra at iuuk dot mff.cuni.cz> 2013-04-22 08:32:07 UTC ---
Created attachment 6996
  --> http://sourceware.org/bugzilla/attachment.cgi?id=6996
benchmark for inlines


> finite7 from your "benchmark for inlines" is giving me a different answer for
> 1.0 and 2.0. I would understand it better with 0x7ff instead of 127.
Yes I fixed benchmark, 127 was from writing that in hurry. 
It gives different answer as spec only says it have to return nonzero value. I
wrote mail to ask about that.

-- 
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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[fweimer at redhat dot com]

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

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/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 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  b8682397ab2db1aed7f25d0a0c7c81134a97c8c7 (commit)
      from  46f74e1deee549b41160d353ce0c8f7db555d36c (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=b8682397ab2db1aed7f25d0a0c7c81134a97c8c7

commit b8682397ab2db1aed7f25d0a0c7c81134a97c8c7
Author: Joseph Myers <joseph@codesourcery.com>
Date:   Thu Sep 17 16:47:14 2015 +0000

    Reduce number of constants in __finite* (bug 15384).

    Bug 15384 notes that in __finite, two different constants are used
    that could be the same constant (the result only depends on the
    exponent of the floating-point representation), and that using the
    same constant is better for architectures where constants need loading
    from a constant pool.  This patch implements that change.

    Tested for x86_64, mips64 and powerpc.

        [BZ #15384]
        * sysdeps/ieee754/dbl-64/s_finite.c (FINITE): Use same constant as
        bit-mask as in subtraction.
        * sysdeps/ieee754/dbl-64/wordsize-64/s_finite.c (__finite):
        Likewise.
        * sysdeps/ieee754/flt-32/s_finitef.c (FINITEF): Likewise.
        * sysdeps/ieee754/ldbl-128/s_finitel.c (__finitel): Likewise.
        * sysdeps/ieee754/ldbl-128ibm/s_finitel.c (__finitel): Likewise.

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

Summary of changes:
 ChangeLog                                     |    9 +++++++++
 NEWS                                          |   14 +++++++-------
 sysdeps/ieee754/dbl-64/s_finite.c             |    2 +-
 sysdeps/ieee754/dbl-64/wordsize-64/s_finite.c |    2 +-
 sysdeps/ieee754/flt-32/s_finitef.c            |    2 +-
 sysdeps/ieee754/ldbl-128/s_finitel.c          |    2 +-
 sysdeps/ieee754/ldbl-128ibm/s_finitel.c       |    2 +-
 7 files changed, 21 insertions(+), 12 deletions(-)

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

[Bug math/15384] One constant fewer in ieee754/dbl-64/wordsize-64/s_finite.c

[jsm28 at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|---                         |2.23

--- Comment #11 from Joseph Myers <jsm28 at gcc dot gnu.org> ---
Fixed for 2.23.

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