From: Linus Torvalds Subject: Re: linux/bitops.h Date: Wed, 4 May 2016 17:48:40 -0700 Message-ID: References: <1462170413-7164-1-git-send-email-tytso@mit.edu> <1462170413-7164-2-git-send-email-tytso@mit.edu> <20160504174901.GC3901@thunk.org> <20160504190723.GD3901@thunk.org> <572A6CDD.10503@av8n.com> <572A724C.6010704@av8n.com> <572A9437.4020208@zytor.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: John Denker , "Theodore Ts'o" , noloader@gmail.com, Linux Kernel Mailing List , Stephan Mueller , Herbert Xu , Andi Kleen , Sandy Harris , cryptography@lakedaemon.net, Linux Crypto Mailing List , Sasha Levin To: "H. Peter Anvin" Return-path: In-Reply-To: <572A9437.4020208@zytor.com> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-crypto.vger.kernel.org On Wed, May 4, 2016 at 5:30 PM, H. Peter Anvin wrote: > > Yes. d7e35dfa is baloney IMNSHO. All it does is produce worse code, and the > description even says so. > > As I said, gcc has treated the former code as idiomatic since gcc 2, so that > support is beyond ancient. Well, we're *trying* to get clang supported, so the argument that gcc has always supported it and compiles correct code for it is not necessarily the whole story yet. The problem with "32 - shift" is that it really could generate garbage in situations where shift ends up being a constant zero.. That said, the fact that the other cases weren't changed (rol64/ror64/ror32) does make that argument less interesting. Unless there was some particular code that actively ended up using "rol32(..0)" but not the other cases. (And yes, rol32(x,0) is obviously nonsense, but it could easily happen with inline functions or macros that end up generating that). Note that there may be 8 "rol/ror" functions, but the 16-bit and 8-bit ones don't have the undefined semantics. So there are only four that had _that_ problem, although I agree that changing just one out of four is wrong. Linus