From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x331.google.com (mail-wm1-x331.google.com [IPv6:2a00:1450:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id 13A943858C3A; Sat, 27 Nov 2021 18:24:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 13A943858C3A Received: by mail-wm1-x331.google.com with SMTP id o29so10927466wms.2; Sat, 27 Nov 2021 10:24:15 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=9wG+RQy//A1jWs+ffmf5CFkn71BtG7ajiuoJE4X2w2k=; b=Jx3ft4gymeA1hA9/14neBeTc0/0Bm03V6ZAicp/DcNUEcTed/FUiRnwE0zZp687LVs SQJ7/NjzM3Oye2jlsHgjYVRtD64Cew4Z/qQrHI7Aw8BpN9qgVrhyoYGK9kkHxnZFpOhm airU4Et9UkkBzvXehNc4b7Rj2lmcPnCD8vCBhNWaWnRCVkww1vauffgM4OMu2QX+aD6J gR+jesoqthL0c/6TRdKe28+aIgd+4613fHhYh97bUj2XsOZ5NiASC00hYO6JukBxlbLQ 9uU0+h+ucEd9pTcoPKYI5dxBDdYA7lptGUMLoQIwPYMmjc1gxK7jGvpWn3lBmZXiTZvR hfMQ== X-Gm-Message-State: AOAM531xWGMQeOnvHtFs0snU4jNX/PxH0vobU1YPZSNzjF+8fWpa2a3l pNupu5VVSG7kP1gQMKKp9zB/GCa/iIOeJh9CrSY= X-Google-Smtp-Source: ABdhPJw2jOfAFT6/qXZwmDZ+dK8i2DpX3W9A7kKWQ2l5uVAzUIwl9oPNNQDdyJfKYZDW3eN81XY4qHaUQv9kh+H7GNc= X-Received: by 2002:a1c:e918:: with SMTP id q24mr24759662wmc.108.1638037454076; Sat, 27 Nov 2021 10:24:14 -0800 (PST) MIME-Version: 1.0 References: <20211127115956.GV2646553@tucnak> <20211127121922.GW2646553@tucnak> In-Reply-To: <20211127121922.GW2646553@tucnak> From: Jonathan Wakely Date: Sat, 27 Nov 2021 18:23:58 +0000 Message-ID: Subject: Re: [PATCH] libstdc++, v2: Implement std::byteswap for C++23 To: Jakub Jelinek Cc: gcc-patches , "libstdc++" X-Spam-Status: No, score=-0.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, HTML_MESSAGE, KAM_NUMSUBJECT, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libstdc++@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libstdc++ mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 27 Nov 2021 18:24:25 -0000 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 > > * 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 > + constexpr enable_if_t::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(~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 > --- 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 > +// . > + > +// { dg-options "-std=gnu++2b" } > +// { dg-do compile { target c++23 } } > + > +#include > + > +#ifndef __cpp_lib_byteswap > +# error "Feature-test macro for byteswap missing in " > +#elif __cpp_lib_byteswap != 202110L > +# error "Feature-test macro for byteswap has wrong value in " > +#endif > + > +#include > +#include > +#include > + > +void > +test01() > +{ > + static_assert( std::byteswap(0x12) == 0x12 ); > + static_assert( std::byteswap(0x1234) == 0x3412 ); > + static_assert( std::byteswap(0x12345678) == 0x78563412 ); > + static_assert( std::byteswap(0x123456789abcdef0) > + == static_cast(0xf0debc9a78563412) ); > + static_assert( std::byteswap(0x21) == 0x21 ); > + static_assert( std::byteswap(0x4321) == 0x2143 ); > + static_assert( std::byteswap(0x87654321) == 0x21436587 ); > + static_assert( std::byteswap(0xfedcba9876543210) > + == static_cast(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(0xdeadbeef) == 0xefbeadde > ); > + static_assert( std::byteswap(0xdeadbeef) == > 0xefbeadde ); > + static_assert( std::byteswap(0xdeadbeef) > + == static_cast(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(a) == 0x12 ); > + VERIFY ( std::byteswap(b) == 0x3412 ); > + VERIFY ( std::byteswap(c) == 0x78563412 ); > + VERIFY ( std::byteswap(d) == 0xf0debc9a78563412 ); > + VERIFY ( std::byteswap(e) == 0x21 ); > + VERIFY ( std::byteswap(f) == 0x2143 ); > + VERIFY ( std::byteswap(g) == 0x21436587 ); > + VERIFY ( std::byteswap(h) == 0x1032547698badcfe ); > + VERIFY ( std::byteswap(std::byteswap(a)) == a ); > + VERIFY ( std::byteswap(std::byteswap(b)) == b ); > + VERIFY ( std::byteswap(std::byteswap(c)) == c ); > + VERIFY ( std::byteswap(std::byteswap(d)) == d ); > + VERIFY ( std::byteswap(std::byteswap(e)) == e ); > + VERIFY ( std::byteswap(std::byteswap(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(0xdeadbeef) == 0xefbeadde ); > + VERIFY ( std::byteswap(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 > +// . > + > +// { dg-options "-std=gnu++2b" } > +// { dg-do compile { target c++23 } } > + > +#include > + > +#ifndef __cpp_lib_byteswap > +# error "Feature-test macro for bit_cast missing in " > +#elif __cpp_lib_byteswap != 202110L > +# error "Feature-test macro for byteswap has wrong value in " > +#endif > > > Jakub > >