Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH v1 bitops] bitops: Fix UBSAN undefined behavior warning
 for rotation right
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        Andrew Morton <akpm@linux-foundation.org>,
        Vadim Pasternak <vadimp@mellanox.com>
Cc:     jacek.anaszewski@gmail.com, pavel@ucw.cz,
        linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org,
        idosch@mellanox.com, Andrey Ryabinin <aryabinin@virtuozzo.com>
References: <20190407125325.24440-1-vadimp@mellanox.com>
 <20190408155217.3f723554ae7fbcb34eeacb30@linux-foundation.org>
 <f38907e5-82a4-1e9f-a3a4-b4e366dbb90b@rasmusvillemoes.dk>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
Message-ID: <b631316b-74bc-7d19-c6a4-70b5459b44cc@rasmusvillemoes.dk>
Date:   Tue, 9 Apr 2019 10:57:32 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <f38907e5-82a4-1e9f-a3a4-b4e366dbb90b@rasmusvillemoes.dk>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 09/04/2019 10.08, Rasmus Villemoes wrote:

> one could do
> 
> u32 ror32(u32 x, unsigned s)
> {
> 	return (x >> (s&31)) | (x << ((32-s)&31));
> }
> 
> to make the shifts always well-defined and also work as expected for s
> >= 32... if only gcc recognized that the masking is redundant, so that
> its "that's a ror" pattern detection could kick in. Unfortunately, it
> seems that the above generates
> 
>    0:   89 f1                   mov    %esi,%ecx
>    2:   89 f8                   mov    %edi,%eax
>    4:   f7 d9                   neg    %ecx
>    6:   d3 e0                   shl    %cl,%eax
>    8:   89 f1                   mov    %esi,%ecx
>    a:   d3 ef                   shr    %cl,%edi
>    c:   09 f8                   or     %edi,%eax
>    e:   c3                      retq
> 
> while without the masking one gets
> 
>   10:   89 f8                   mov    %edi,%eax
>   12:   89 f1                   mov    %esi,%ecx
>   14:   d3 c8                   ror    %cl,%eax
>   16:   c3                      retq

Ah, but that's with an ancient gcc 7. With gcc 8, the above pattern is
recognized and generates good code, while eliminating UB. I was about to
file a gcc bug, but found
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82498 .

Rasmus