Re: [PATCH] block-sha1: remove use of assembly

["brian m. carlson" <sandals@crustytoothpaste.net>]

[-- Attachment #1: Type: text/plain, Size: 1866 bytes --]

On 2022-03-07 at 23:32:42, Taylor Blau wrote:
> On Mon, Mar 07, 2022 at 11:25:52PM +0000, brian m. carlson wrote:
> > In the block SHA-1 code, we have special assembly code for i386 and
> > amd64 to perform rotations with assembly.  This is supposed to help pick
> > the correct rotation operation depending on which rotation is smaller,
> > which can help some systems perform slightly better, since any circular
> > rotation can be specified as either a rotate left or a rotate right.
> > However, this isn't needed, so we should remove it.
> 
> At -O1 or higher (at least on GCC) this optimization is indeed
> performed. Here's a Godbolt example that shows this:
> 
>     https://godbolt.org/z/9zMP93hr1
> 
> so I agree that this code isn't helping us at all. And in the
> meantime...

Thanks for providing a link.  I also did a similar test there with
slightly different code (and unfortunately closed the window before
saving the link) but it demonstrated the same thing: that the compiler
can optimize this case adequately.  My (substantially) past experience
with testing GCC in this case has shown the same thing.

> > The downside of using this code, however, is that it uses a GCC
> > extension, which makes the compiler complain when using -pedantic unless
> > it's prefixed with __extension__.  We could fix that, but since it's
> > not needed, let's just remove it.  We haven't noticed this because
> > almost everyone uses the SHA1DC code instead, but it still shows up for
> > some people.
> 
> ...it makes it impossible to compile git if you have
> `BLK_SHA1=YesPlease` and `DEVELOPER=1` in your environment. So I am
> happy to see this go.
> 
> On another note: missing Signed-off-by?

I'll send an otherwise unchanged v2 with that in a second.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]