[Bug libstdc++/58625] New: std::signbit always converts to double

[Bug libstdc++/58625] New: std::signbit always converts to double

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

            Bug ID: 58625
           Summary: std::signbit always converts to double
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: olegendo at gcc dot gnu.org

In file libstdc++-v4/include/c_global:

  constexpr bool
  signbit(float __x)
  { return __builtin_signbit(__x); }

  constexpr bool
  signbit(double __x)
  { return __builtin_signbit(__x); }

  constexpr bool
  signbit(long double __x)
  { return __builtin_signbit(__x); }


I believe this is supposed to be:

  constexpr bool
  signbit(float __x)
  { return __builtin_signbitf(__x); }

  constexpr bool
  signbit(double __x)
  { return __builtin_signbit(__x); }

  constexpr bool
  signbit(long double __x)
  { return __builtin_signbitl(__x); }

As it is right now, std::signbit always does a conversion to double.

[Bug libstdc++/58625] std::signbit always converts to double

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Weird that nobody noticed for so much time. Are there actual testcases for
this? Or in practice it's just an optimization issue, not a correctness issue?

Now I suspect isinf and isnan too, for which we would not have available *f and
*l counterparts at all. Could you please check?

I suppose we, libstdc++-v3 people, simply assumed that the C++ front-end
provided a full set of intrinsics corresponding to C99 classification macros,
besides those for the comparison macros.

Adding Jakub in CC: the change per se is so easy, but maybe I'm missing some
background for this.

[Bug libstdc++/58625] std::signbit always converts to double

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-10-04
     Ever confirmed|0                           |1

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Well, now I see this can be certainly a correctness issue, for eg very small
negative long doubles. Thus I think we should certainly do the change anyway
for 4.9. For c_std I'm afraid we have to add overloads (I think, essentially
copy over the corresponding bits of c_global without constexpr). Can you send a
patch to the libstdc++-v3 mailing list? And, please, double check isnan and
isinf. Thanks!

[Bug libstdc++/58625] std::signbit always converts to double

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Paolo Carlini from comment #1)
> Weird that nobody noticed for so much time. Are there actual testcases for
> this? Or in practice it's just an optimization issue, not a correctness
> issue?

In some cases there could be a correctness issue with some rounding modes
though not that many.

[Bug libstdc++/58625] std::signbit always converts to double

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Some builtins are type generic, meaning that you can use just the non-{l,f}
suffixed variant always, but __builtin_signbit isn't among them.
Grep shows that these builtins have "type generic" attribute:

DEF_GCC_BUILTIN        (BUILT_IN_FPCLASSIFY, "fpclassify",
BT_FN_INT_INT_INT_INT_INT_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISFINITE, "isfinite", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISINF_SIGN, "isinf_sign", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_C99_C90RES_BUILTIN (BUILT_IN_ISINF, "isinf", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC)
DEF_C99_C90RES_BUILTIN (BUILT_IN_ISNAN, "isnan", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISNORMAL, "isnormal", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISGREATER, "isgreater", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISGREATEREQUAL, "isgreaterequal",
BT_FN_INT_VAR, ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISLESS, "isless", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISLESSEQUAL, "islessequal", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISLESSGREATER, "islessgreater", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)
DEF_GCC_BUILTIN        (BUILT_IN_ISUNORDERED, "isunordered", BT_FN_INT_VAR,
ATTR_CONST_NOTHROW_TYPEGENERIC_LEAF)

[Bug libstdc++/58625] std::signbit always converts to double

[olegendo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #5)
> Some builtins are type generic, meaning that you can use just the non-{l,f}
> suffixed variant always, but __builtin_signbit isn't among them.

Yep.  So libstdc++-v3/include/c_std/cmath (signbit) would require a fix, too in
this case, right?  Or maybe rather make the signbit builtin type generic?

[Bug libstdc++/58625] std::signbit always converts to double

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I don't think it is a good idea to extend the number of type generic builtins
unless strictly necessary, the overloading in C is quite ugly hack.

[Bug libstdc++/58625] std::signbit always converts to double

[jakub at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
That said, I'd say that every conversion to double that would change the sign
looks wrong to me, no matter of what the rounding mode is, except perhaps for
NaN canonicalization and that sNaN could be signalled.  So IMHO this is mostly
an optimization thing, not correctness.

[Bug libstdc++/58625] std::signbit always converts to double

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Ok, thanks a lot for the comments. Thus, Oleg, given in particular the last
comment from Jakub, let's just apply to mainline your initial proposed change,
let's leave for now include/c_std alone. Just test the change, send it to the
mailing lists as preapproved. Please also add a short comment before the code,
explaining that a type generic signbit builtin isn't available. Thanks!

[Bug libstdc++/58625] std::signbit always converts to double

[daniel.kruegler at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #10 from Daniel Krügler <daniel.kruegler at googlemail dot com> ---
(In reply to Oleg Endo from comment #4)
> There's another place in file libstdc++-v3/include/c_std/cmath:
> 
>   template<typename _Tp>
>     inline typename __gnu_cxx::__enable_if<__is_arithmetic<_Tp>::__value,
> 					   int>::__type
>     signbit(_Tp __f)
>     {
>       typedef typename __gnu_cxx::__promote<_Tp>::__type __type;
>       return __builtin_signbit(__type(__f));
>     }

I think this template should no longer exist after resolution of

http://cplusplus.github.io/LWG/lwg-defects.html#1327

(The P/R doesn't become very clear in the quoted reference. In essence it
replaced the previous template by the three overloads mentioned above, see
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3289.pdf
)
>From gcc-bugs-return-431138-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Sat Oct 05 12:12:10 2013
Return-Path: <gcc-bugs-return-431138-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 15826 invoked by alias); 5 Oct 2013 12:12:10 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 15793 invoked by uid 48); 5 Oct 2013 12:12:07 -0000
From: "paolo.carlini at oracle dot com" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/58629] Do not implicit move instance for throw operator
Date: Sat, 05 Oct 2013 12:12:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c++
X-Bugzilla-Version: 4.7.3
X-Bugzilla-Keywords:
X-Bugzilla-Severity: normal
X-Bugzilla-Who: paolo.carlini at oracle dot com
X-Bugzilla-Status: UNCONFIRMED
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: ---
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields:
Message-ID: <bug-58629-4-LjESdyyVMk@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-58629-4@http.gcc.gnu.org/bugzilla/>
References: <bug-58629-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-10/txt/msg00282.txt.bz2
Content-length: 149

http://gcc.gnu.org/bugzilla/show_bug.cgi?idX629

--- Comment #2 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Please compare c++/57533

[Bug libstdc++/58625] std::signbit always converts to double

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #11 from Paolo Carlini <paolo.carlini at oracle dot com> ---
At some point, as we discussed already a bit, we should even try to remove
completely the c_std version. For now as I said, let's just leave it alone.

[Bug libstdc++/58625] std::signbit always converts to double

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #13 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> ---
Author: paolo
Date: Sun Oct  6 13:44:47 2013
New Revision: 203228

URL: http://gcc.gnu.org/viewcvs?rev=203228&root=gcc&view=rev
Log:
2013-10-06  Oleg Endo  <olegendo@gcc.gnu.org>
        Paolo Carlini  <paolo.carlini@oracle.com>

    PR libstdc++/58625
    * include/c_global/cmath (signbit): Use __builtin_signbitf and
    __builtin_signbitl.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/c_global/cmath

[Bug libstdc++/58625] std::signbit always converts to double

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED
           Assignee|paolo.carlini at oracle dot com    |unassigned at gcc dot gnu.org

--- Comment #14 from Paolo Carlini <paolo.carlini at oracle dot com> ---
Done.

[Bug libstdc++/58625] std::signbit always converts to double

[tijl at coosemans dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

Tijl Coosemans <tijl at coosemans dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tijl at coosemans dot org

--- Comment #15 from Tijl Coosemans <tijl at coosemans dot org> ---
(In reply to Jakub Jelinek from comment #8)
> That said, I'd say that every conversion to double that would change the
> sign looks wrong to me, no matter of what the rounding mode is, except
> perhaps for NaN canonicalization and that sNaN could be signalled.  So IMHO
> this is mostly an optimization thing, not correctness.

I do think this is a correctness problem for long double.  Consider the
following C code:

int test( long double x ) {
        return( __builtin_signbit( x ));
}

With "gcc49 -O2 -S test.c" on x86_64 this compiles to:

fldt      8(%rsp)
fstpl     -16(%rsp)
movsd     -16(%rsp), %xmm0
movmskpd  %xmm0, %eax
andl      $1, %eax
ret

The fstpl instruction converts long double to double and may generate all kinds
of floating point exceptions if the value can't be converted exactly and I
don't think signbit(x) is allowed to generate FP exceptions.  So it would be
best to fix the c_std version too.

[Bug libstdc++/58625] std::signbit always converts to double

[joseph at codesourcery dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58625

--- Comment #16 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
That C __builtin_signbit should be type-generic is bug 36757.