On Sat, 2 Nov 2024 at 07:11, Jakub Jelinek <ja...@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 <ja...@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
>
