This is the mail archive of the libstdc++@gcc.gnu.org mailing list for the libstdc++ project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] P0556R3 Integral power-of-2 operations, P0553R2 Bit operations


On 04/07/18 10:26 +0200, Jakub Jelinek wrote:
On Wed, Jul 04, 2018 at 09:14:04AM +0100, Jonathan Wakely wrote:
> Unfortunately it is not correct if _Nd is not power of two.
> E.g. for __sN 1, -1U % 20 is 15, not 19.
> So it would need to be
>      return (__x << __sN) | (__x >> ((_Nd - __sN) % _Nd));
> Unfortunately, our rotate pattern recognizer handles
>      return (__x << __sN) | (__x >> ((-__sN) % _Nd));
> or
>      return (__x << __sN) | (__x >> ((-__sN) & (_Nd - 1)));
> but doesn't handle the _Nd - __sN case.
> Is this C++17+ only?  Then perhaps

The std::rotr and std::rotl functions are C++2a only, but I've added
the __rotr and __rotl versions for our own internal use in C++14 and
later.

In practice I have no internal use for rotr and rotl, so I could
remove the __rot[rl] forms. However, won't ((_Nd & (_Nd - 1)) optimize
to a constant even without if-constexpr? I'll check.

It should, sure.

Anyway, I guess my C tests didn't test properly what happens in your
functions.  We actually do optimize:
unsigned long long foo (unsigned long long __x, unsigned int __s)
{
 constexpr int _Nd = 64;
 unsigned int _sN = __s % _Nd;
 return (__x << _sN) | (__x >> ((_Nd - _sN) % _Nd));
}
properly, just don't do it if it is:
unsigned long long foo (unsigned long long __x, unsigned int __s)
{
 int _Nd = 64;
 unsigned int _sN = __s % _Nd;
 return (__x << _sN) | (__x >> ((_Nd - _sN) % _Nd));
}
Apparently, the (64 - x) & 63 optimization to -x & 63 is only done
somewhere in the FEs and not in match.pd, which is something we should fix.

But with constexpr _Nd you actually can use
     return (__x << __sN) | (__x >> ((_Nd - __sN) % _Nd));
unconditionally.

OK here's what I've just tested and committed to trunk.


commit 35640efe0911095ad6ce38771e40a22a6cbb1221
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Wed Jul 4 14:37:57 2018 +0100

    Fix std::__rotl and std::__rotr
    
    2018-07-04  Jonathan Wakely  <jwakely@redhat.com>
                Jakub Jelinek  <jakub@redhat.com>
    
            * include/std/bit (__rotl, __rotr): Fix for non-power of two sizes.

diff --git a/libstdc++-v3/include/std/bit b/libstdc++-v3/include/std/bit
index ace88954030..a23f2ba60d1 100644
--- a/libstdc++-v3/include/std/bit
+++ b/libstdc++-v3/include/std/bit
@@ -46,7 +46,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       constexpr auto _Nd = numeric_limits<_Tp>::digits;
       const unsigned __sN = __s % _Nd;
-      return (__x << __sN) | (__x >> ((-__sN) % _Nd));
+      return (__x << __sN) | (__x >> ((_Nd - __sN) % _Nd));
     }
 
   template<typename _Tp>
@@ -55,7 +55,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       constexpr auto _Nd = numeric_limits<_Tp>::digits;
       const unsigned __sN = __s % _Nd;
-      return (__x >> __sN) | (__x << ((-__sN) % _Nd));
+      return (__x >> __sN) | (__x << ((_Nd - __sN) % _Nd));
     }
 
   template<typename _Tp>

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]