diff mbox series

libstdc++: Fix up std::{,b}float16_t std::{ilogb,l{,l}r{ound,int}} [PR117406]

Message ID ZyXQZE29hpBdCUc6@tucnak
State New
Headers show
Series libstdc++: Fix up std::{,b}float16_t std::{ilogb,l{,l}r{ound,int}} [PR117406] | expand

Commit Message

Jakub Jelinek Nov. 2, 2024, 7:10 a.m. UTC
Hi!

These overloads incorrectly cast the result of the float __builtin_*
to _Float or __gnu_cxx::__bfloat16_t.  For std::ilogb that changes
behavior for the INT_MAX return because that isn't representable in
either of the floating point formats, for the others it is I think
just a very inefficient hop from int/long/long long to std::{,b}float16_t
and back.  I mean for the round/rint cases, either the argument is small
and then the return value should be representable in the floating point
format too, or it is too large that the argument is already integral
and then it should just return the argument with the round trips.
Too large value is unspecified unlike ilogb.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2024-11-02  Jakub Jelinek  <jakub@redhat.com>

	PR libstdc++/117406
	* include/c_global/cmath (std::ilogb(_Float16), std::llrint(_Float16),
	std::llround(_Float16), std::lrint(_Float16), std::lround(_Float16)):
	Don't cast __builtin_* return to _Float16.
	(std::ilogb(__gnu_cxx::__bfloat16_t),
	std::llrint(__gnu_cxx::__bfloat16_t),
	std::llround(__gnu_cxx::__bfloat16_t),
	std::lrint(__gnu_cxx::__bfloat16_t),
	std::lround(__gnu_cxx::__bfloat16_t)): Don't cast __builtin_* return to
	__gnu_cxx::__bfloat16_t.
	* testsuite/26_numerics/headers/cmath/117406.cc: New test.


	Jakub

Comments

Jonathan Wakely Nov. 2, 2024, 1:32 p.m. UTC | #1
On Sat, 2 Nov 2024 at 07:11, Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> These overloads incorrectly cast the result of the float __builtin_*
> to _Float or __gnu_cxx::__bfloat16_t.  For std::ilogb that changes
> behavior for the INT_MAX return because that isn't representable in
> either of the floating point formats, for the others it is I think
> just a very inefficient hop from int/long/long long to std::{,b}float16_t
> and back.  I mean for the round/rint cases, either the argument is small
> and then the return value should be representable in the floating point
> format too, or it is too large that the argument is already integral
> and then it should just return the argument with the round trips.
> Too large value is unspecified unlike ilogb.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK for trunk, thanks.

I think it's worth backporting too.

>
> 2024-11-02  Jakub Jelinek  <jakub@redhat.com>
>
>         PR libstdc++/117406
>         * include/c_global/cmath (std::ilogb(_Float16), std::llrint(_Float16),
>         std::llround(_Float16), std::lrint(_Float16), std::lround(_Float16)):
>         Don't cast __builtin_* return to _Float16.
>         (std::ilogb(__gnu_cxx::__bfloat16_t),
>         std::llrint(__gnu_cxx::__bfloat16_t),
>         std::llround(__gnu_cxx::__bfloat16_t),
>         std::lrint(__gnu_cxx::__bfloat16_t),
>         std::lround(__gnu_cxx::__bfloat16_t)): Don't cast __builtin_* return to
>         __gnu_cxx::__bfloat16_t.
>         * testsuite/26_numerics/headers/cmath/117406.cc: New test.
>
> --- libstdc++-v3/include/c_global/cmath.jj      2024-10-29 11:13:47.741803660 +0100
> +++ libstdc++-v3/include/c_global/cmath 2024-11-01 21:11:40.367678439 +0100
> @@ -2838,7 +2838,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr int
>    ilogb(_Float16 __x)
> -  { return _Float16(__builtin_ilogbf(__x)); }
> +  { return __builtin_ilogbf(__x); }
>
>    constexpr _Float16
>    lgamma(_Float16 __x)
> @@ -2846,11 +2846,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr long long
>    llrint(_Float16 __x)
> -  { return _Float16(__builtin_llrintf(__x)); }
> +  { return __builtin_llrintf(__x); }
>
>    constexpr long long
>    llround(_Float16 __x)
> -  { return _Float16(__builtin_llroundf(__x)); }
> +  { return __builtin_llroundf(__x); }
>
>    constexpr _Float16
>    log1p(_Float16 __x)
> @@ -2867,11 +2867,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr long
>    lrint(_Float16 __x)
> -  { return _Float16(__builtin_lrintf(__x)); }
> +  { return __builtin_lrintf(__x); }
>
>    constexpr long
>    lround(_Float16 __x)
> -  { return _Float16(__builtin_lroundf(__x)); }
> +  { return __builtin_lroundf(__x); }
>
>    constexpr _Float16
>    nearbyint(_Float16 __x)
> @@ -3560,7 +3560,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr int
>    ilogb(__gnu_cxx::__bfloat16_t __x)
> -  { return __gnu_cxx::__bfloat16_t(__builtin_ilogbf(__x)); }
> +  { return __builtin_ilogbf(__x); }
>
>    constexpr __gnu_cxx::__bfloat16_t
>    lgamma(__gnu_cxx::__bfloat16_t __x)
> @@ -3568,11 +3568,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr long long
>    llrint(__gnu_cxx::__bfloat16_t __x)
> -  { return __gnu_cxx::__bfloat16_t(__builtin_llrintf(__x)); }
> +  { return __builtin_llrintf(__x); }
>
>    constexpr long long
>    llround(__gnu_cxx::__bfloat16_t __x)
> -  { return __gnu_cxx::__bfloat16_t(__builtin_llroundf(__x)); }
> +  { return __builtin_llroundf(__x); }
>
>    constexpr __gnu_cxx::__bfloat16_t
>    log1p(__gnu_cxx::__bfloat16_t __x)
> @@ -3589,11 +3589,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>
>    constexpr long
>    lrint(__gnu_cxx::__bfloat16_t __x)
> -  { return __gnu_cxx::__bfloat16_t(__builtin_lrintf(__x)); }
> +  { return __builtin_lrintf(__x); }
>
>    constexpr long
>    lround(__gnu_cxx::__bfloat16_t __x)
> -  { return __gnu_cxx::__bfloat16_t(__builtin_lroundf(__x)); }
> +  { return __builtin_lroundf(__x); }
>
>    constexpr __gnu_cxx::__bfloat16_t
>    nearbyint(__gnu_cxx::__bfloat16_t __x)
> --- libstdc++-v3/testsuite/26_numerics/headers/cmath/117406.cc.jj       2024-11-02 07:55:07.555121231 +0100
> +++ libstdc++-v3/testsuite/26_numerics/headers/cmath/117406.cc  2024-11-02 08:00:14.150679537 +0100
> @@ -0,0 +1,59 @@
> +// Copyright (C) 2024 Free Software Foundation, Inc.
> +//
> +// This file is part of the GNU ISO C++ Library.  This library is free
> +// software; you can redistribute it and/or modify it under the
> +// terms of the GNU General Public License as published by the
> +// Free Software Foundation; either version 3, or (at your option)
> +// any later version.
> +
> +// This library is distributed in the hope that it will be useful,
> +// but WITHOUT ANY WARRANTY; without even the implied warranty of
> +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +// GNU General Public License for more details.
> +
> +// You should have received a copy of the GNU General Public License along
> +// with this library; see the file COPYING3.  If not see
> +// <http://www.gnu.org/licenses/>.
> +
> +// { dg-do run { target c++23 } }
> +// { dg-require-cmath "" }
> +
> +#include <stdfloat>
> +#include <cmath>
> +#include <limits>
> +#include <testsuite_hooks.h>
> +
> +template <typename T>
> +void
> +test ()
> +{
> +  using lim = std::numeric_limits<T>;
> +  int t0 = std::ilogb(T(4.0));
> +  VERIFY( t0 == 2 );
> +  int t1 = std::ilogb(lim::infinity());
> +  VERIFY( t1 == INT_MAX );
> +  int t2 = std::ilogb(-lim::infinity());
> +  VERIFY( t2 == INT_MAX );
> +}
> +
> +int
> +main ()
> +{
> +#if defined(__STDCPP_FLOAT16_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
> +  test <std::float16_t>();
> +#endif
> +#if defined(__STDCPP_FLOAT32_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
> +  test <std::float32_t>();
> +#endif
> +#if defined(__STDCPP_FLOAT64_T__) && defined(_GLIBCXX_DOUBLE_IS_IEEE_BINARY64)
> +  test <std::float64_t>();
> +#endif
> +#if defined(__STDCPP_FLOAT128_T__) \
> +    && (defined(_GLIBCXX_LDOUBLE_IS_IEEE_BINARY128) \
> +       || defined(_GLIBCXX_HAVE_FLOAT128_MATH))
> +  test <std::float128_t>();
> +#endif
> +#if defined(__STDCPP_BFLOAT16_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
> +  test <std::bfloat16_t>();
> +#endif
> +}
>
>         Jakub
>
diff mbox series

Patch

--- libstdc++-v3/include/c_global/cmath.jj	2024-10-29 11:13:47.741803660 +0100
+++ libstdc++-v3/include/c_global/cmath	2024-11-01 21:11:40.367678439 +0100
@@ -2838,7 +2838,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr int
   ilogb(_Float16 __x)
-  { return _Float16(__builtin_ilogbf(__x)); }
+  { return __builtin_ilogbf(__x); }
 
   constexpr _Float16
   lgamma(_Float16 __x)
@@ -2846,11 +2846,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr long long
   llrint(_Float16 __x)
-  { return _Float16(__builtin_llrintf(__x)); }
+  { return __builtin_llrintf(__x); }
 
   constexpr long long
   llround(_Float16 __x)
-  { return _Float16(__builtin_llroundf(__x)); }
+  { return __builtin_llroundf(__x); }
 
   constexpr _Float16
   log1p(_Float16 __x)
@@ -2867,11 +2867,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr long
   lrint(_Float16 __x)
-  { return _Float16(__builtin_lrintf(__x)); }
+  { return __builtin_lrintf(__x); }
 
   constexpr long
   lround(_Float16 __x)
-  { return _Float16(__builtin_lroundf(__x)); }
+  { return __builtin_lroundf(__x); }
 
   constexpr _Float16
   nearbyint(_Float16 __x)
@@ -3560,7 +3560,7 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr int
   ilogb(__gnu_cxx::__bfloat16_t __x)
-  { return __gnu_cxx::__bfloat16_t(__builtin_ilogbf(__x)); }
+  { return __builtin_ilogbf(__x); }
 
   constexpr __gnu_cxx::__bfloat16_t
   lgamma(__gnu_cxx::__bfloat16_t __x)
@@ -3568,11 +3568,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr long long
   llrint(__gnu_cxx::__bfloat16_t __x)
-  { return __gnu_cxx::__bfloat16_t(__builtin_llrintf(__x)); }
+  { return __builtin_llrintf(__x); }
 
   constexpr long long
   llround(__gnu_cxx::__bfloat16_t __x)
-  { return __gnu_cxx::__bfloat16_t(__builtin_llroundf(__x)); }
+  { return __builtin_llroundf(__x); }
 
   constexpr __gnu_cxx::__bfloat16_t
   log1p(__gnu_cxx::__bfloat16_t __x)
@@ -3589,11 +3589,11 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   constexpr long
   lrint(__gnu_cxx::__bfloat16_t __x)
-  { return __gnu_cxx::__bfloat16_t(__builtin_lrintf(__x)); }
+  { return __builtin_lrintf(__x); }
 
   constexpr long
   lround(__gnu_cxx::__bfloat16_t __x)
-  { return __gnu_cxx::__bfloat16_t(__builtin_lroundf(__x)); }
+  { return __builtin_lroundf(__x); }
 
   constexpr __gnu_cxx::__bfloat16_t
   nearbyint(__gnu_cxx::__bfloat16_t __x)
--- libstdc++-v3/testsuite/26_numerics/headers/cmath/117406.cc.jj	2024-11-02 07:55:07.555121231 +0100
+++ libstdc++-v3/testsuite/26_numerics/headers/cmath/117406.cc	2024-11-02 08:00:14.150679537 +0100
@@ -0,0 +1,59 @@ 
+// Copyright (C) 2024 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do run { target c++23 } }
+// { dg-require-cmath "" }
+
+#include <stdfloat>
+#include <cmath>
+#include <limits>
+#include <testsuite_hooks.h>
+
+template <typename T>
+void
+test ()
+{
+  using lim = std::numeric_limits<T>;
+  int t0 = std::ilogb(T(4.0));
+  VERIFY( t0 == 2 );
+  int t1 = std::ilogb(lim::infinity());
+  VERIFY( t1 == INT_MAX );
+  int t2 = std::ilogb(-lim::infinity());
+  VERIFY( t2 == INT_MAX );
+}
+
+int
+main ()
+{
+#if defined(__STDCPP_FLOAT16_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
+  test <std::float16_t>();
+#endif
+#if defined(__STDCPP_FLOAT32_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
+  test <std::float32_t>();
+#endif
+#if defined(__STDCPP_FLOAT64_T__) && defined(_GLIBCXX_DOUBLE_IS_IEEE_BINARY64)
+  test <std::float64_t>();
+#endif
+#if defined(__STDCPP_FLOAT128_T__) \
+    && (defined(_GLIBCXX_LDOUBLE_IS_IEEE_BINARY128) \
+	|| defined(_GLIBCXX_HAVE_FLOAT128_MATH))
+  test <std::float128_t>();
+#endif
+#if defined(__STDCPP_BFLOAT16_T__) && defined(_GLIBCXX_FLOAT_IS_IEEE_BINARY32)
+  test <std::bfloat16_t>();
+#endif
+}