On Thu, 20 Feb 2025 at 14:41, Patrick Palka <ppa...@redhat.com> wrote:
>
> On Sat, 15 Feb 2025, Jonathan Wakely wrote:
>
> > These should have been unsigned, but the static assertions are only in
> > the public std::bit_ceil and std::bit_width functions, not the internal
> > __bit_ceil and __bit_width ones.
> >
> > libstdc++-v3/ChangeLog:
> >
> > * include/experimental/bits/simd.h (__find_next_valid_abi): Cast
> > __bit_ceil argument to unsigned.
> > * src/c++17/floating_from_chars.cc (__floating_from_chars_hex):
> > Cast __bit_ceil argument to unsigned.
> > * src/c++17/memory_resource.cc (big_block): Cast __bit_width
> > argument to unsigned.
> > ---
> >
> > Tested x86_64-linux.
> >
> > libstdc++-v3/include/experimental/bits/simd.h | 2 +-
> > libstdc++-v3/src/c++17/floating_from_chars.cc | 5 +++--
> > libstdc++-v3/src/c++17/memory_resource.cc | 3 ++-
> > 3 files changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/libstdc++-v3/include/experimental/bits/simd.h
> > b/libstdc++-v3/include/experimental/bits/simd.h
> > index 500c1781ae54..f0cb101aaa82 100644
> > --- a/libstdc++-v3/include/experimental/bits/simd.h
> > +++ b/libstdc++-v3/include/experimental/bits/simd.h
> > @@ -4634,7 +4634,7 @@ template <template <int> class _Abi, int _Bytes,
> > typename _Tp>
> > static constexpr auto
> > _S_choose()
> > {
> > - constexpr int _NextBytes = std::__bit_ceil(_Bytes) / 2;
> > + constexpr int _NextBytes = std::__bit_ceil((unsigned)_Bytes) / 2;
> > using _NextAbi = _Abi<_NextBytes>;
> > if constexpr (_NextBytes < sizeof(_Tp) * 2) // break recursion
> > return _Abi<_Bytes>();
> > diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc
> > b/libstdc++-v3/src/c++17/floating_from_chars.cc
> > index f8b1e23937d2..d48f1c0d4545 100644
> > --- a/libstdc++-v3/src/c++17/floating_from_chars.cc
> > +++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
> > @@ -1102,8 +1102,9 @@ namespace
> > {
> > // If the leading hexit is not '1', shift MANTISSA to make it so.
> > // This normalizes input like "4.08p0" into "1.02p2".
> > - const int leading_hexit = mantissa >> mantissa_bits;
> > - const int leading_hexit_width = __bit_width(leading_hexit); // FIXME:
> > optimize?
> > + const unsigned leading_hexit = mantissa >> mantissa_bits;
> > + const int leading_hexit_width
> > + = __bit_width((unsigned)leading_hexit); // FIXME: optimize?
>
> Not a big deal, but isn't this cast redundant if we've defined
> leading_hexit as unsigned?
You can never be too careful in C++! ;-)
Yes you're absolutely right. I think I added the cast at first, then
realised that I could just make the variable unsigned, but forgot to
undo the cast.
The attached patch reverts the cast part, just leaving the
s/int/unsigned/ part on the previous line.
commit ef1655d3c3caeac56cc9124a2479a5bfac90a239
Author: Jonathan Wakely <jwak...@redhat.com>
AuthorDate: Thu Feb 20 15:16:11 2025
Commit: Jonathan Wakely <r...@gcc.gnu.org>
CommitDate: Thu Feb 20 15:16:11 2025
libstdc++: Remove redundant cast in floating_from_chars.cc
In r15-7647-g32457bc25fea80 I added a cast and also changed the type of
the variable, making the cast redundant. This removes the cast.
libstdc++-v3/ChangeLog:
* src/c++17/floating_from_chars.cc (__floating_from_chars_hex):
Remove redundant cast.
diff --git a/libstdc++-v3/src/c++17/floating_from_chars.cc
b/libstdc++-v3/src/c++17/floating_from_chars.cc
index d48f1c0d454..9bad12c7f69 100644
--- a/libstdc++-v3/src/c++17/floating_from_chars.cc
+++ b/libstdc++-v3/src/c++17/floating_from_chars.cc
@@ -1103,8 +1103,7 @@ namespace
// If the leading hexit is not '1', shift MANTISSA to make it so.
// This normalizes input like "4.08p0" into "1.02p2".
const unsigned leading_hexit = mantissa >> mantissa_bits;
- const int leading_hexit_width
- = __bit_width((unsigned)leading_hexit); // FIXME: optimize?
+ const int leading_hexit_width = __bit_width(leading_hexit); // FIXME:
optimize?
__glibcxx_assert(leading_hexit_width >= 1 && leading_hexit_width <= 4);
shift_mantissa(leading_hexit_width - 1);
// After this adjustment, we can assume the leading hexit is '1'.
