Re: [PATCH] libstdc++, v2: Implement std::byteswap for C++23

[Jonathan Wakely <jwakely.gcc@gmail.com>]

On Sat, 27 Nov 2021, 12:20 Jakub Jelinek via Libstdc++, <
libstdc++@gcc.gnu.org> wrote:

> On Sat, Nov 27, 2021 at 12:59:56PM +0100, Jakub Jelinek via Gcc-patches
> wrote:
> > This patch attempts to implement P1272R4 (except for the std::bit_cast
> > changes in there which seem quite unrelated to this and will need to be
> > fixed on the compiler side).
> > While at least for GCC __builtin_bswap{16,32,64,128} should work fine
> > in constant expressions, I wonder about other compilers, so I'm using
> > a fallback implementation for constexpr evaluation always.
> > If you think that is unnecessary, I can drop the
> > __cpp_if_consteval >= 202106L &&
> > if !consteval
> >   {
> > and
> >   }
> > and reformat.
>

Let's keep it, we can simplify it later if we verify it works everywhere.



> The fallback implementation is an attempt to make it work even for
> integral
> > types that don't have number of bytes divisible by 2 or when __CHAR_BIT__
> > is e.g. 16.
>
> Or here is a slightly different version of the fallback implementation that
> should do slightly fewer operations per cycle (I guess it matters primarily
> for constexpr evaluation).  Also tested on x86_64-linux.
>

Even better.






> 2021-11-27  Jakub Jelinek  <jakub@redhat.com>
>
>         * include/std/bit (__cpp_lib_byteswap, byteswap): Define.
>         * include/std/version (__cpp_lib_byteswap): Define.
>         * testsuite/26_numerics/bit/bit.byteswap/byteswap.cc: New test.
>         * testsuite/26_numerics/bit/bit.byteswap/version.cc: New test.
>
> --- libstdc++-v3/include/std/bit.jj     2021-04-13 20:41:50.781976767 +0200
> +++ libstdc++-v3/include/std/bit        2021-11-27 13:12:52.498769700 +0100
> @@ -78,6 +78,55 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
>      }
>  #endif
>
> +#if __cplusplus > 202002l
>

Uppercase L please.

Then it's OK for trunk, thanks.

+#define __cpp_lib_byteswap 202110L
> +
> +  /// Reverse order of bytes in the object representation of `value`.
> +  template<typename _Tp>
> +    constexpr enable_if_t<is_integral<_Tp>::value, _Tp>
> +    byteswap(_Tp __value) noexcept
> +    {
> +#if __cpp_if_consteval >= 202106L && __CHAR_BIT__ == 8
> +      if !consteval
> +       {
> +         if constexpr (sizeof(_Tp) == 1)
> +           return __value;
> +         if constexpr (sizeof(_Tp) == 2)
> +           return __builtin_bswap16(__value);
> +         if constexpr (sizeof(_Tp) == 4)
> +           return __builtin_bswap32(__value);
> +         if constexpr (sizeof(_Tp) == 8)
> +           return __builtin_bswap64(__value);
> +         if constexpr (sizeof(_Tp) == 16)
> +#if __has_builtin(__builtin_bswap128)
> +           return __builtin_bswap128(__value);
> +#else
> +           return (__builtin_bswap64(__value >> 64)
> +                   | (__builtin_bswap64(__value) << 64));
> +#endif
> +       }
> +#endif
> +
> +      // Fallback implementation that handles even __int24 etc.
> +      using _Up = typename __make_unsigned<__remove_cv_t<_Tp>>::__type;
> +      size_t __diff = __CHAR_BIT__ * (sizeof(_Tp) - 1);
> +      _Up __mask1 = static_cast<unsigned char>(~0);
> +      _Up __mask2 = __mask1 << __diff;
> +      _Up __val = __value;
> +      for (size_t __i = 0; __i < sizeof(_Tp) / 2; ++__i)
> +       {
> +         _Up __byte1 = __val & __mask1;
> +         _Up __byte2 = __val & __mask2;
> +         __val = (__val ^ __byte1 ^ __byte2
> +                  ^ (__byte1 << __diff) ^ (__byte2 >> __diff));
> +         __mask1 <<= __CHAR_BIT__;
> +         __mask2 >>= __CHAR_BIT__;
> +         __diff -= 2 * __CHAR_BIT__;
> +       }
> +      return __val;
> +    }
> +#endif
> +
>    /// @cond undoc
>
>    template<typename _Tp>
> --- libstdc++-v3/include/std/version.jj 2021-11-17 10:10:08.034949063 +0100
> +++ libstdc++-v3/include/std/version    2021-11-27 10:50:44.620238052 +0100
> @@ -294,6 +294,7 @@
>  #if __cplusplus > 202002L
>  // c++2b
>  #define __cpp_lib_adaptor_iterator_pair_constructor 202106L
> +#define __cpp_lib_byteswap 202110L
>  #define __cpp_lib_invoke_r 202106L
>  #define __cpp_lib_is_scoped_enum 202011L
>  #if __cpp_lib_concepts
> --- libstdc++-v3/testsuite/26_numerics/bit/bit.byteswap/byteswap.cc.jj
> 2021-11-27 11:40:50.654149348 +0100
> +++ libstdc++-v3/testsuite/26_numerics/bit/bit.byteswap/byteswap.cc
>  2021-11-27 12:43:00.696889120 +0100
> @@ -0,0 +1,112 @@
> +// Copyright (C) 2021 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-options "-std=gnu++2b" }
> +// { dg-do compile { target c++23 } }
> +
> +#include <bit>
> +
> +#ifndef __cpp_lib_byteswap
> +# error "Feature-test macro for byteswap missing in <bit>"
> +#elif __cpp_lib_byteswap != 202110L
> +# error "Feature-test macro for byteswap has wrong value in <bit>"
> +#endif
> +
> +#include <cstdint>
> +#include <cstring>
> +#include <testsuite_hooks.h>
> +
> +void
> +test01()
> +{
> +  static_assert( std::byteswap<int8_t>(0x12) == 0x12 );
> +  static_assert( std::byteswap<int16_t>(0x1234) == 0x3412 );
> +  static_assert( std::byteswap<int32_t>(0x12345678) == 0x78563412 );
> +  static_assert( std::byteswap<int64_t>(0x123456789abcdef0)
> +                == static_cast<int64_t>(0xf0debc9a78563412) );
> +  static_assert( std::byteswap<uint8_t>(0x21) == 0x21 );
> +  static_assert( std::byteswap<uint16_t>(0x4321) == 0x2143 );
> +  static_assert( std::byteswap<uint32_t>(0x87654321) == 0x21436587 );
> +  static_assert( std::byteswap<uint64_t>(0xfedcba9876543210)
> +                == static_cast<uint64_t>(0x1032547698badcfe) );
> +#if !defined(__STRICT_ANSI__) && defined __SIZEOF_INT128__
> +  constexpr __int128_t c1 = (static_cast<__int128_t>(0x0102030405060708)
> << 64
> +                            | 0x090a0b0c0d0e0f10);
> +  constexpr __int128_t c2 = (static_cast<__int128_t>(0x100f0e0d0c0b0a09)
> << 64
> +                            | 0x0807060504030201);
> +  constexpr __int128_t c3 = (static_cast<__int128_t>(0xf1e2d3c4b5a69788)
> << 64
> +                            | 0x796a5b4c3d2e1f10);
> +  constexpr __int128_t c4 = (static_cast<__int128_t>(0x101f2e3d4c5b6a79)
> << 64
> +                            | 0x8897a6b5c4d3e2f1);
> +  static_assert( std::byteswap(c1) == c2 );
> +  static_assert( std::byteswap(static_cast<__uint128_t>(c1))
> +                == static_cast<__uint128_t>(c2) );
> +  static_assert( std::byteswap(c3) == c4 );
> +#endif
> +  static_assert( std::byteswap<const uint32_t>(0xdeadbeef) == 0xefbeadde
> );
> +  static_assert( std::byteswap<volatile uint32_t>(0xdeadbeef) ==
> 0xefbeadde );
> +  static_assert( std::byteswap<int32_t>(0xdeadbeef)
> +                == static_cast<int32_t>(0xefbeadde) );
> +}
> +
> +void
> +test02()
> +{
> +  volatile int8_t a = 0x12;
> +  volatile int16_t b = 0x1234;
> +  volatile int32_t c = 0x12345678;
> +  volatile int64_t d = 0x123456789abcdef0;
> +  volatile uint8_t e = 0x21;
> +  volatile uint16_t f = 0x4321;
> +  volatile uint32_t g = 0x87654321;
> +  volatile uint64_t h = 0xfedcba9876543210;
> +  VERIFY ( std::byteswap<int8_t>(a) == 0x12 );
> +  VERIFY ( std::byteswap<int16_t>(b) == 0x3412 );
> +  VERIFY ( std::byteswap(c) == 0x78563412 );
> +  VERIFY ( std::byteswap(d) == 0xf0debc9a78563412 );
> +  VERIFY ( std::byteswap<uint8_t>(e) == 0x21 );
> +  VERIFY ( std::byteswap<uint16_t>(f) == 0x2143 );
> +  VERIFY ( std::byteswap(g) == 0x21436587 );
> +  VERIFY ( std::byteswap(h) == 0x1032547698badcfe );
> +  VERIFY ( std::byteswap(std::byteswap<int8_t>(a)) == a );
> +  VERIFY ( std::byteswap(std::byteswap<int16_t>(b)) == b );
> +  VERIFY ( std::byteswap(std::byteswap(c)) == c );
> +  VERIFY ( std::byteswap(std::byteswap(d)) == d );
> +  VERIFY ( std::byteswap(std::byteswap<uint8_t>(e)) == e );
> +  VERIFY ( std::byteswap(std::byteswap<uint16_t>(f)) == f );
> +  VERIFY ( std::byteswap(std::byteswap(g)) == g );
> +  VERIFY ( std::byteswap(std::byteswap(h)) == h );
> +#if !defined(__STRICT_ANSI__) && defined __SIZEOF_INT128__
> +  volatile __int128_t c1 = (static_cast<__int128_t>(0x0102030405060708)
> << 64
> +                           | 0x090a0b0c0d0e0f10);
> +  volatile __int128_t c2 = (static_cast<__int128_t>(0x100f0e0d0c0b0a09)
> << 64
> +                           | 0x0807060504030201);
> +  VERIFY ( std::byteswap(c1) == c2 );
> +  VERIFY ( std::byteswap<__uint128_t>(c1) == static_cast<__uint128_t>(c2)
> );
> +  VERIFY ( std::byteswap(std::byteswap(c1)) == c1 );
> +  VERIFY ( std::byteswap(std::byteswap<__uint128_t>(c2))
> +          == static_cast<__uint128_t>(c2) );
> +#endif
> +  VERIFY ( std::byteswap<const uint32_t>(0xdeadbeef) == 0xefbeadde );
> +  VERIFY ( std::byteswap<volatile uint32_t>(0xdeadbeef) == 0xefbeadde );
> +}
> +
> +int main()
> +{
> +  test01();
> +  test02();
> +}
> --- libstdc++-v3/testsuite/26_numerics/bit/bit.byteswap/version.cc.jj
>  2021-11-27 11:39:48.741024747 +0100
> +++ libstdc++-v3/testsuite/26_numerics/bit/bit.byteswap/version.cc
> 2021-11-27 11:40:36.048355861 +0100
> @@ -0,0 +1,27 @@
> +// Copyright (C) 2021 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-options "-std=gnu++2b" }
> +// { dg-do compile { target c++23 } }
> +
> +#include <version>
> +
> +#ifndef __cpp_lib_byteswap
> +# error "Feature-test macro for bit_cast missing in <version>"
> +#elif __cpp_lib_byteswap != 202110L
> +# error "Feature-test macro for byteswap has wrong value in <version>"
> +#endif
>
>
>         Jakub
>
>