Re: [PATCH 2/4] sha1dc-accel: vectorize the unavoidable-bitconditions check
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Oct 7, 2026, 21:32 UTC
- Message-ID
- <xmqqh5ix5h8l.fsf@gitster.g>
- In-Reply-To
- <3d640489-5db4-5527-0ec1-c2abac7a2de3@gmx.de>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 24 quoted lines
> This Perl script reproduces the tables (although with different > formatting, and without the inline comments, I verified it with > `--patience --color-words="[A-Za-z0-9_]+|."`). > ... > With all that out of the way, I would like to ask to include this script > in the patch (or in a follow-up patch) so that the lengthy `ubc_check.c` > file's tables can be validated/regenerated independently. > ... > While this code is correct, I think it is slightly misleading: depending > on `want`, it either subtracts `set` from `dvs`, or takes the minimum. But > that only happens to be what is desired because each lane of `set` is all > ones or all zero. What we actually want is to mask either those lanes or > everything but those lanes, i.e. `dvs & ~set` or `dvs & set`, > respectively. That would be: > > fail = g->want ? vbicq_u32(dvs, set) : vandq_u32(dvs, set); > > This has no speed impact nor does it produce a "more correct" result, but > it might improve readability a bit. > > I haven't looked very closely whether there are similar issues elsewhere > (it is relatively tedious for me to learn all this NEON stuff on the go, > this is all new to me). If you're familiar with NEON, it might be > worthwhile looking for similarly "correct but misleading" statements.
Thanks for offering a very thoughtful help and offering to work well together.