Repository navigation
Return CRYPTO_memcmp's accumulator through a value barrier - #13
Conversation
Clang 23 folds the conversion of the uint8_t accumulator to int into the loop (llvm/llvm-project#222142). The accumulator becomes 32 bits wide, and the loop vectorizer then puts 4 bytes in a 128-bit vector instead of 16. A 1 KB compare takes about 120 ns instead of about 26 ns. value_barrier_w takes a crypto_word_t, which is wider than int, so the fold does not apply and the accumulator stays 8 bits wide. The result is also opaque to the compiler where CRYPTO_memcmp is inlined.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review. Walkthrough
ChangesConstant-time comparison
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The change preserves comparison semantics while improving compiler-generated performance, with no established merge-blocking risk. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
oven-sh/bun#43885 pins this commit (51a84a8) as a preview, so bun's CI builds and tests it on every platform. It is a draft until this PR merges. Measured there on linux x64 release builds (ns per call, Bun 1.4.2 / main / with this commit): |
|
bun's CI passed with this commit pinned (oven-sh/bun#43885, Buildkite build 120273): 181 of 181 jobs. That covers the build and the full test suite on linux x64 and aarch64 (glibc, musl, android, ASAN), darwin x64 and aarch64, freebsd x64 and aarch64, windows x64 and aarch64, plus the baseline instruction scans. |
2 participants
Problem
CRYPTO_memcmpis 3 to 5 times slower when clang 23 compiles it: 120 ns for 1 KB against 26 ns. In Bun,crypto.timingSafeEqualandKeyObject.equalson 1 KB inputs went from 40 ns to 130 ns after the LLVM 23 upgrade (Upgrade LLVM 21.1.8 → 23.1.1 and Rust nightly to 2026-09-15 bun#42851).x |= a[i] ^ b[i]computes inint, andreturn xconvertsxtointagain. Clang 23 folds that conversion into the loop, keeps a 32-bit accumulator, and vectorizes with 4 x i32 lanes (8 bytes per iteration) instead of 16 x i8 (32 bytes per iteration).Fix
xthroughvalue_barrier_w. Acrypto_word_tis wider thanint, so the fold does not apply and the accumulator stays 8 bits wide.CRYPTO_memcmpis inlined.-march=nehalem) and arm64: the old loop returns. Verified in a Bun linux x64 release build with ThinLTO:KeyObject.equalson 1 KB keys goes from 130 ns to 37 ns (31 ns on Bun 1.4.2).Background
CRYPTO_memcmpis the constant-time compare behind AEAD tag checks and TLS MACs. In Bun it also servescrypto.timingSafeEqual,KeyObject.equals, CSRF tokens and the Postgres SCRAM check.value_barrier_w(crypto/internal.h) returns its argument through an empty inline-asm statement. The compiler cannot reason about the value, but it still vectorizes the loop.scripts/build/deps/boringssl.ts. A bun PR bumps the pin after this merges.Notes
In the Bun release build,
CRYPTO_memcmpdisassembles tomovdqu/pxor/poragain, andKeyObject.equalson 64 KB keys goes from 9.0 µs to 2.3 µs (2.2 µs on 1.4.2). Without the barrier, x64 getspmovzxbd+poron 4 x i32 and arm64 gets 16tblper 64 bytes.Scalar IR from clang 23 without the barrier:
%x = phi i32,zext i8 ... to i32,or i32. With the barrier:%x = phi i8,or i8, and onezext i8 ... to i64after the loop. The fold in llvm/llvm-project#222142 iszext(trunc nuw X) -> X. It needs the extended type to match the type ofX, which isinthere. On a 32-bit targetcrypto_word_tis as wide asint, so this change does not help there. Bun builds 64-bit targets only.Upstream BoringSSL has the same source and the same slowdown under clang 23.