Re: [PATCH 0/4] faster SHA-1 collision detection
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Oct 7, 2026, 12:17 UTC
- Message-ID
- <76b7e19d-c9f9-0a0a-6154-a797b64cf9b0@gmx.de>
- In-Reply-To
- <20260929112544.86511-1-scott@gitbutler.net>
Hi Scott,
On Tue, 29 Sep 2026, Scott Chacon wrote:
Show 6 quoted lines
> So, spoiler alert, the code in this patch series is mainly AI generated. > I would try to fool you, but too many of you are far too aware of my > actual C skills. That being said, I thought maybe someone here (especially > those of you working on server optimization stuff) would be interested > in the speed increases for both the server and client in making sha1dc > quite a bit faster.
Hah, I beat you by almost a full day with my "competing" series at https://lore.kernel.org/git/pull.2240.git.1790610691.gitgitgadget@gmail.com/. I put the "competing" in double quotes because I think that both patch series have merit, and I would love to see both merged.
Performance-wise, in my tests the Rust `sha1dc` was a tad faster that this C-accell code on my Ryzen, something like 3.3% for my favorite `index-pack` benchmark with 30 randomized pairings. Naturally, I tried to figure out where that difference comes from, but I haven't been able to finish that analysis to my satisfaction, although I would like to offer this patch to avoid reordering the message schedule, which closes the gap from 3.3% to 1%:
-- snip -- From: Johannes Schindelin <johannes.schindelin@gmx.de> Date: Sun, 4 Oct 2026 12:53:56 +0200 Subject: [PATCH] sha1dc-accel: bring SHA-NI performance closer to Rust
The C SHA-NI path spends instructions reordering the message schedule, while Rust uses a mirrored schedule. Avoid that overhead without changing the collision checks.
Across 30 randomized, AC-powered triplets on the same pack, C's mean fell from 8.201s to 8.053s. The paired mean improvement was 0.147s (95% CI: 0.083-0.211s), leaving C about 1% behind Rust.
Assisted-by: GPT-6 Sol Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- sha1dc-accel/internal.h | 7 +++- sha1dc-accel/sha1.c | 22 +++++++++-- sha1dc-accel/ubc_check.c | 82 ++++++++++++++++++++++++++++++---------- sha1dc-accel/x86.c | 4 +- t/unit-tests/u-sha1dc.c | 34 +++++++++++++---- 5 files changed, 112 insertions(+), 37 deletions(-)
diff --git a/sha1dc-accel/internal.h b/sha1dc-accel/internal.h index a50b294b96b..bcb60811a8f 100644 --- a/sha1dc-accel/internal.h +++ b/sha1dc-accel/internal.h @@ -4,8 +4,9 @@ /* * Shared between the files of sha1dc-accel/. See sha1.c for an overview. * - * The schedule `w` is always the 80 expanded message words in step order, - * w[t] at index t, which is also what sha1dc/ keeps in SHA1_CTX.m1. + * The schedule `w` has 80 expanded message words. Normally w[t] is at + * index t, as in sha1dc/. SHA-NI spills w[t] at index 79 - t; a candidate + * is put back in step order before recompression. * * A "state" is the five working words [a, b, c, d, e] before a step. */ @@ -75,9 +76,11 @@ enum sha1dc_from { uint32_t sha1dc_ubc_check_scalar(const uint32_t w[80]); #ifdef SHA1DC_HAVE_SSE2 uint32_t sha1dc_ubc_check_sse2(const uint32_t w[80]); +uint32_t sha1dc_ubc_check_sse2_mirrored(const uint32_t w[80]); #endif #ifdef SHA1DC_HAVE_AVX2 uint32_t sha1dc_ubc_check_avx2(const uint32_t w[80]); +uint32_t sha1dc_ubc_check_avx2_mirrored(const uint32_t w[80]); #endif #ifdef SHA1DC_HAVE_NEON uint32_t sha1dc_ubc_check_neon(const uint32_t w[80]); diff --git a/sha1dc-accel/sha1.c b/sha1dc-accel/sha1.c index 1f1133169b5..bddc3bcdf2d 100644 --- a/sha1dc-accel/sha1.c +++ b/sha1dc-accel/sha1.c @@ -259,9 +259,11 @@ static int shani_avx2_available(void) /* In order of preference. */ static const struct backend backends[] = { #ifdef SHA1DC_HAVE_SHANI - { "shani+avx2", sha1dc_compress_shani, 1, sha1dc_ubc_check_avx2, + { "shani+avx2", sha1dc_compress_shani, 1, + sha1dc_ubc_check_avx2_mirrored, sha1dc_recompress_shani, shani_avx2_available }, - { "shani+sse2", sha1dc_compress_shani, 1, sha1dc_ubc_check_sse2, + { "shani+sse2", sha1dc_compress_shani, 1, + sha1dc_ubc_check_sse2_mirrored, sha1dc_recompress_shani, sha1dc_shani_available }, #endif #ifdef SHA1DC_HAVE_ARMV8 @@ -432,8 +434,20 @@ static inline void process(const struct backend *be, SHA1_CTX *ctx, return; candidates = ctx->ubc_check ? be->ubc_check(w) : 0xFFFFFFFF; - if (candidates && - attacked(be, ctx, candidates, w, s1, s2, ihv_in, ctx->ihv)) { + if (!candidates) + return; +#ifdef SHA1DC_HAVE_SHANI + if (be->compress == sha1dc_compress_shani) { + int i; + + for (i = 0; i < 40; i++) { + uint32_t tmp = w[i]; + w[i] = w[79 - i]; + w[79 - i] = tmp; + } + } +#endif + if (attacked(be, ctx, candidates, w, s1, s2, ihv_in, ctx->ihv)) { ctx->found_collision = 1; /* * Two more compressions of this block give a digest that the diff --git a/sha1dc-accel/ubc_check.c b/sha1dc-accel/ubc_check.c index f95b799f9d4..cbd21d33e68 100644 --- a/sha1dc-accel/ubc_check.c +++ b/sha1dc-accel/ubc_check.c @@ -105,9 +105,13 @@ struct ubc_cond { uint8_t i, a, j, b, c; }; -static inline uint32_t cond_fails(const uint32_t *w, const struct ubc_cond *c) +static inline uint32_t cond_fails(const uint32_t *w, const struct ubc_cond *c, + int mirrored) { - return (((w[c->i] >> c->a) ^ (w[c->j] >> c->b) ^ c->c) & 1); + unsigned i = mirrored ? 79 - c->i : c->i; + unsigned j = mirrored ? 79 - c->j : c->j; + + return (((w[i] >> c->a) ^ (w[j] >> c->b) ^ c->c) & 1); } /* A condition of the scalar prefix, and the DVs it rules out if it fails. */ @@ -149,7 +153,7 @@ struct tail_span { */ static inline uint32_t run_tail(const uint32_t *w, uint32_t mask, const struct ubc_cond *checks, - const struct tail_span *spans) + const struct tail_span *spans, int mirrored) { uint32_t out = mask; while (mask) { @@ -160,7 +164,7 @@ static inline uint32_t run_tail(const uint32_t *w, uint32_t mask, NO_VECTORIZE for (; c < end; c++) - fail |= cond_fails(w, c); + fail |= cond_fails(w, c, mirrored); out &= ~(fail << d); mask &= mask - 1; } @@ -569,7 +573,7 @@ static uint32_t scalar_prefix(const uint32_t *w) UNROLL_TABLE for (i = 0; i < ARRAY_SIZE(scalar_prefix_conds); i++) { const struct ubc_prefix_cond *p = &scalar_prefix_conds[i]; - mask &= ~(p->dvs & (0 - cond_fails(w, &p->cond))); + mask &= ~(p->dvs & (0 - cond_fails(w, &p->cond, 0))); } return mask; } @@ -580,7 +584,7 @@ uint32_t sha1dc_ubc_check_scalar(const uint32_t w[80]) /* Every check only clears bits, so an empty mask settles it. */ if (!mask) return 0; - return run_tail(w, mask, scalar_tail_checks, scalar_tail_spans); + return run_tail(w, mask, scalar_tail_checks, scalar_tail_spans, 0); } /* neon form */ @@ -980,7 +984,7 @@ uint32_t sha1dc_ubc_check_neon(const uint32_t w[80]) /* Every check only clears bits, so an empty mask settles it. */ if (!mask) return 0; - return run_tail(w, mask, neon_tail_checks, neon_tail_spans); + return run_tail(w, mask, neon_tail_checks, neon_tail_spans, 0); } #endif /* SHA1DC_HAVE_NEON */ @@ -1343,7 +1347,7 @@ static const struct tail_span sse2_tail_spans[32] = { }; SHA1DC_TARGET_SSE2 -static uint32_t sse2_prefix(const uint32_t *w) +static inline uint32_t sse2_prefix(const uint32_t *w, int mirrored) { const __m128i zero = _mm_setzero_si128(); __m128i acc = zero; @@ -1352,10 +1356,18 @@ static uint32_t sse2_prefix(const uint32_t *w) UNROLL_TABLE for (i = 0; i < ARRAY_SIZE(sse2_groups); i++) { const struct ubc_group4 *g = &sse2_groups[i]; - __m128i lo = _mm_loadu_si128((const __m128i *)(w + g->lo)); - __m128i hi = _mm_loadu_si128((const __m128i *)(w + g->hi)); - __m128i test = _mm_loadu_si128((const __m128i *)g->test); - __m128i dvs = _mm_loadu_si128((const __m128i *)g->dvs); + const uint32_t *lo_w = w + (mirrored ? 76 - g->lo : g->lo); + const uint32_t *hi_w = w + (mirrored ? 76 - g->hi : g->hi); + __m128i lo = _mm_loadu_si128((const __m128i *)lo_w); + __m128i hi = _mm_loadu_si128((const __m128i *)hi_w); + __m128i test = mirrored ? + _mm_set_epi32(g->test[0], g->test[1], + g->test[2], g->test[3]) : + _mm_loadu_si128((const __m128i *)g->test); + __m128i dvs = mirrored ? + _mm_set_epi32(g->dvs[0], g->dvs[1], + g->dvs[2], g->dvs[3]) : + _mm_loadu_si128((const __m128i *)g->dvs); __m128i clear, fail; lo = _mm_srl_epi32(lo, _mm_cvtsi32_si128(g->lo_shift)); @@ -1376,11 +1388,20 @@ static uint32_t sse2_prefix(const uint32_t *w) SHA1DC_TARGET_SSE2 uint32_t sha1dc_ubc_check_sse2(const uint32_t w[80]) { - uint32_t mask = sse2_prefix(w); + uint32_t mask = sse2_prefix(w, 0); /* Every check only clears bits, so an empty mask settles it. */ if (!mask) return 0; - return run_tail(w, mask, sse2_tail_checks, sse2_tail_spans); + return run_tail(w, mask, sse2_tail_checks, sse2_tail_spans, 0); +} + +SHA1DC_TARGET_SSE2 +uint32_t sha1dc_ubc_check_sse2_mirrored(const uint32_t w[80]) +{ + uint32_t mask = sse2_prefix(w, 1); + if (!mask) + return 0; + return run_tail(w, mask, sse2_tail_checks, sse2_tail_spans, 1); } #endif /* SHA1DC_HAVE_SSE2 */ @@ -1743,7 +1764,7 @@ static const struct tail_span avx2_tail_spans[32] = { }; SHA1DC_TARGET_AVX2 -static uint32_t avx2_prefix(const uint32_t *w) +static inline uint32_t avx2_prefix(const uint32_t *w, int mirrored) { const __m256i zero = _mm256_setzero_si256(); __m256i acc = zero; @@ -1753,10 +1774,20 @@ static uint32_t avx2_prefix(const uint32_t *w) UNROLL_TABLE for (i = 0; i < ARRAY_SIZE(avx2_groups); i++) { const struct ubc_group8 *g = &avx2_groups[i]; - __m256i lo = _mm256_loadu_si256((const __m256i *)(w + g->lo)); - __m256i hi = _mm256_loadu_si256((const __m256i *)(w + g->hi)); - __m256i test = _mm256_loadu_si256((const __m256i *)g->test); - __m256i dvs = _mm256_loadu_si256((const __m256i *)g->dvs); + const uint32_t *lo_w = w + (mirrored ? 72 - g->lo : g->lo); + const uint32_t *hi_w = w + (mirrored ? 72 - g->hi : g->hi); + __m256i lo = _mm256_loadu_si256((const __m256i *)lo_w); + __m256i hi = _mm256_loadu_si256((const __m256i *)hi_w); + __m256i test = mirrored ? + _mm256_set_epi32(g->test[0], g->test[1], g->test[2], + g->test[3], g->test[4], g->test[5], + g->test[6], g->test[7]) : + _mm256_loadu_si256((const __m256i *)g->test); + __m256i dvs = mirrored ? + _mm256_set_epi32(g->dvs[0], g->dvs[1], g->dvs[2], + g->dvs[3], g->dvs[4], g->dvs[5], + g->dvs[6], g->dvs[7]) : + _mm256_loadu_si256((const __m256i *)g->dvs); __m256i clear, fail; lo = _mm256_srl_epi32(lo, _mm_cvtsi32_si128(g->lo_shift)); @@ -1779,11 +1810,20 @@ static uint32_t avx2_prefix(const uint32_t *w) SHA1DC_TARGET_AVX2 uint32_t sha1dc_ubc_check_avx2(const uint32_t w[80]) { - uint32_t mask = avx2_prefix(w); + uint32_t mask = avx2_prefix(w, 0); /* Every check only clears bits, so an empty mask settles it. */ if (!mask) return 0; - return run_tail(w, mask, avx2_tail_checks, avx2_tail_spans); + return run_tail(w, mask, avx2_tail_checks, avx2_tail_spans, 0); +} + +SHA1DC_TARGET_AVX2 +uint32_t sha1dc_ubc_check_avx2_mirrored(const uint32_t w[80]) +{ + uint32_t mask = avx2_prefix(w, 1); + if (!mask) + return 0; + return run_tail(w, mask, avx2_tail_checks, avx2_tail_spans, 1); } #endif /* SHA1DC_HAVE_AVX2 */ diff --git a/sha1dc-accel/x86.c b/sha1dc-accel/x86.c index c2a0c71f17f..dfdc1fc3b62 100644 --- a/sha1dc-accel/x86.c +++ b/sha1dc-accel/x86.c @@ -66,8 +66,8 @@ int sha1dc_shani_available(void) #define LOADU(p) _mm_loadu_si128((const __m128i *)(const void *)(p)) #define STOREU(p, v) _mm_storeu_si128((__m128i *)(void *)(p), (v)) -/* Writes group `v` (steps t..t+3, held reversed) to w[t..t+3]. */ -#define SPILL(t, v) STOREU(w + (t), _mm_shuffle_epi32((v), REVERSE)) +/* The group is already reversed; store it in the mirrored schedule. */ +#define SPILL(t, v) STOREU(w + 76 - (t), (v)) /* Schedule words 4k..4k+3 for k from 8 on, from groups k-8, k-7, k-4, k-2, k-1. */ SHA1DC_TARGET_SHANI diff --git a/t/unit-tests/u-sha1dc.c b/t/unit-tests/u-sha1dc.c index f629f59d1d6..c658c219a3a 100644 --- a/t/unit-tests/u-sha1dc.c +++ b/t/unit-tests/u-sha1dc.c @@ -151,6 +151,21 @@ static void check_ubc_forms(uint32_t w[80], int flip, int avx2) check_form(sha1dc_ubc_check_scalar(w), want); #ifdef SHA1DC_HAVE_SSE2 check_form(sha1dc_ubc_check_sse2(w), want); + { + uint32_t mirrored[80]; + int t; + + for (t = 0; t < 80; t++) + mirrored[t] = w[79 - t]; + check_form(sha1dc_ubc_check_sse2_mirrored(mirrored), + want); +#ifdef SHA1DC_HAVE_AVX2 + if (avx2) + check_form( + sha1dc_ubc_check_avx2_mirrored( + mirrored), want); +#endif + } #endif #ifdef SHA1DC_HAVE_AVX2 if (avx2) @@ -243,14 +258,15 @@ typedef int (*recompress_fn)(enum sha1dc_from from, const uint32_t m1[80], const uint32_t dm[80], const uint32_t state[5], const uint32_t ihv_out[5]); -static void check_compress(compress_fn compress) +static void check_compress(compress_fn compress, int mirrored) { int i, t; rng_seed(3); for (i = 0; i < 2000; i++) { unsigned char block[64]; - uint32_t ihv[5], got[5], w[80], at_60[5], at_64[5], s[5]; + uint32_t ihv[5], got[5], w[80], expected[80]; + uint32_t at_60[5], at_64[5], s[5]; for (t = 0; t < 64; t++) block[t] = rng(); @@ -260,14 +276,16 @@ static void check_compress(compress_fn compress) memcpy(s, ihv, sizeof(s)); for (t = 0; t < 80; t++) { - uint32_t want_w = t < 16 ? get_be32(block + 4 * t) : - rol(w[t - 3] ^ w[t - 8] ^ w[t - 14] ^ w[t - 16], 1); - cl_assert_equal_i(w[t], want_w); + expected[t] = t < 16 ? get_be32(block + 4 * t) : + rol(expected[t - 3] ^ expected[t - 8] ^ + expected[t - 14] ^ expected[t - 16], 1); + cl_assert_equal_i(w[mirrored ? 79 - t : t], + expected[t]); if (t == 60) cl_assert(!memcmp(at_60, s, sizeof(s))); if (t == 64) cl_assert(!memcmp(at_64, s, sizeof(s))); - step(s, t, w[t]); + step(s, t, expected[t]); } for (t = 0; t < 5; t++) cl_assert_equal_i(got[t], ihv[t] + s[t]); @@ -321,14 +339,14 @@ static void hardware_compression(void) #ifdef SHA1DC_HAVE_SHANI if (sha1dc_shani_available()) { - check_compress(sha1dc_compress_shani); + check_compress(sha1dc_compress_shani, 1); check_recompress(sha1dc_recompress_shani); tested = 1; } #endif #ifdef SHA1DC_HAVE_ARMV8 if (sha1dc_armv8_available()) { - check_compress(sha1dc_compress_armv8); + check_compress(sha1dc_compress_armv8, 0); check_recompress(sha1dc_recompress_armv8); tested = 1; } -- snap -- This is admittedly a bit gnarly, and really, really hard to understand unless you immersed yourself in Sam's work. But it _does_ accelerate SHA-1 computation with my Ryzen 7, and I'd be interested to hear whether it has an equivalent effect with your Xeon. > This series ports the approach of Sam Reis's sha1dc Rust crate [1], > which gitoxide recently switched to [2], to C. > > The end result hashes roughly 2.7x faster on the Xeon and 2.85x faster > on the M5 Max. Single-threaded index-pack of git.git goes from 24.3s to > 12.7s on the Xeon, and from 16.1s to 8.7s on the M5 Max. > > Hashing throughput on the Xeon, in MiB/s: > > 16KiB 1MiB vs OpenSSL > OpenSSL SHA-1 (no detection) 1234 1129 1.00x > sha1dc/ (today) 435 450 2.67x > shani+avx2 (default here) 1002 901 1.24x > shani+sse2 1075 1008 1.13x > portable+avx2 553 654 1.96x > portable+sse2 603 681 1.84x > portable 466 565 2.29x > > In other words, currently collision detection costs about 1.5–2.5x on > top of the hashing itself today, but only about 0.2x with the series. > > The patches are: > > [1/4]: sha1dc-accel: add a block loop for sha1dc's SHA1_CTX > > Just groundwork: our own block loop around sha1dc's context and DV > table, with the same results and a few percent slower, plus tests > that compare against sha1dc/ directly, including on real collisions > in every mode. > > [2/4]: sha1dc-accel: vectorize the unavoidable-bitconditions check > > The UBC filter rewritten as SSE2, AVX2, NEON, and new scalar forms, > using the conditions the crate's solver picks for each. They're > carried as tables, with a short loop per form to run them. > 1.29x on the Xeon, 1.27x on the M5 Max. > > [3/4]: sha1dc-accel: compress with SHA-NI on x86-64 > > Hardware compression, with the schedule spilled, and recompression > of flagged blocks in hardware, too. Another 2.07x on the Xeon. > > [4/4]: sha1dc-accel: compress with the ARMv8 SHA-1 instructions > > The same for arm64. Another 2.4x on the M5 Max. I really like this structure. BTW I have run the entire test suite both on a Ryzen 7 (using WSL) and on a Windows/ARM64 Cloud PC (using straight Windows because that Cloud PC does not support WSL), with a slightly patched version: running the original (slow) sha1dc, the sha1dc-accel and the Rust sha1dc. There were 0 discrepancies, which meshes with the Sol-assisted analysis of the code, comparing it against the paper and Sam's detailed blog post. I also wanted to compare the speed, but unfortunately, my tests always suffer the noisy neighbor problem (working on a laptop with parallel work going on, or Cloud PC that is of course a VM in a rack somewhere in Virginia). So all I can really claim is that in my tests, the speed was comparable (with the patch above). All that said, I am very much in favor of accepting your patch series into Git, with or without my suggested changes. Thanks! Johannes > > The x86 numbers are from a 4-vCPU Xeon VM with SHA-NI and AVX2 (GCC 13, > Linux), which is unfortunately rather noisy; the per-patch hyperfine > output has the spread. The arm64 numbers are medians of 9 runs on an > Apple M5 Max (Apple clang, macOS). The full test suite passes on both. > > [1] https://sam.dev/blog/faster-sha1-collision-detection > [2] https://github.com/GitoxideLabs/gitoxide/pull/3008 > > Scott Chacon (4): > sha1dc-accel: add a block loop for sha1dc's SHA1_CTX > sha1dc-accel: vectorize the unavoidable-bitconditions check > sha1dc-accel: compress with SHA-NI on x86-64 > sha1dc-accel: compress with the ARMv8 SHA-1 instructions > > Makefile | 14 + > contrib/buildsystems/CMakeLists.txt | 2 +- > meson.build | 4 + > sha1dc-accel/arm.c | 274 ++++ > sha1dc-accel/internal.h | 126 ++ > sha1dc-accel/sha1.c | 498 ++++++++ > sha1dc-accel/sha1.h | 31 + > sha1dc-accel/ubc_check.c | 1789 +++++++++++++++++++++++++++ > sha1dc-accel/x86.c | 260 ++++ > sha1dc_git.c | 18 + > t/.gitattributes | 1 + > t/helper/test-sha1.c | 95 ++ > t/helper/test-tool.c | 2 + > t/helper/test-tool.h | 2 + > t/meson.build | 1 + > t/t0013-sha1dc.sh | 47 + > t/t0013/sha-mbles-1.bin | Bin 0 -> 640 bytes > t/t0013/sha1-reduced-round.bin | Bin 0 -> 128 bytes > t/unit-tests/u-sha1dc.c | 366 ++++++ > 19 files changed, 3529 insertions(+), 1 deletion(-) > create mode 100644 sha1dc-accel/arm.c > create mode 100644 sha1dc-accel/internal.h > create mode 100644 sha1dc-accel/sha1.c > create mode 100644 sha1dc-accel/sha1.h > create mode 100644 sha1dc-accel/ubc_check.c > create mode 100644 sha1dc-accel/x86.c > create mode 100644 t/t0013/sha-mbles-1.bin > create mode 100644 t/t0013/sha1-reduced-round.bin > create mode 100644 t/unit-tests/u-sha1dc.c > > > base-commit: a018953688f1b10bddf91bff8747068f5f4746a4 > -- > 2.50.1 (Apple Git-155) > > > >