Benchmarks only measure the backend the runner's CPU picks, so fallback optimizations (e.g. #142) are invisible

[reaperhulk]

Problem

#142 speeds up the scalar x86-64 GHASH (the PR body says scalar AES-GCM goes from 50 to 109 MiB/s on 16 KiB). Its Benchmarks run (job) shows no change:

Benchmark	Base	Head	Change
aes-128-gcm-encrypt/16384	4.24 µs	4.25 µs	0.3% slower
aes-128-gcm-stream/16384	4.30 µs	4.27 µs	0.6% faster

4.25 µs for 16 KiB is about 3.6 GiB/s, which is the AES-NI + PCLMULQDQ path from #137. The scalar path at 109 MiB/s would take about 140 µs.

Cause: AesGcm::new (src/aes_gcm.rs) picks the backend at run time, and GitHub's x86-64 runners have AES-NI, PCLMULQDQ and SSSE3. So every benchmark runs vg_ghash_pclmul, and the vg_ghash that #142 changes never runs. The comparison is correct (nothing it measured changed), but it measures the wrong code.

The same blind spot already covers SHA-256 and HMAC-SHA-256 on x86-64 (the SHA-NI dispatch in src/hashes/sha256.rs). It will cover every future SIMD or crypto-extension backend: the fallbacks those leave behind get no performance signal at all, and a regression in them would not fail the check either.

Two smaller problems make bulk-cipher results harder to read:

• The summary shows times, not throughput. For a bulk cipher, "4.25 µs" means little without doing the arithmetic. The groups already declare Throughput::Bytes, so MiB/s is free to report.
• Every iteration includes key setup (key expansion plus computing H for GCM). That's the right default ("one complete operation", bench/benches/primitives/main.rs), but at 64 and 1024 bytes setup dominates, and there is no measurement of steady-state per-byte cost.

Plan

The aim is a benchmark of every backend the runner can execute, without making the per-PR Benchmarks job noticeably slower. #142's x86-64 benchmark step took 70 s.

1. Benchmark each backend separately. The API already has __with_features(mask) for tests (AesGcm, AesGcmStream, the hash macro in src/hashes/mod.rs, Hmac). Add a backend to the benchmark id, e.g. aes-128-gcm-encrypt/verified-garbage@scalar/16384 and …@aesni/16384. Skip a backend whose features the runner lacks (cpu::available on the mask), and print that it was skipped rather than silently falling back. bench_compare.py only needs its id parsing extended.
2. Keep the extra backends cheap on PRs. Non-default backends run only the encrypt (or digest) group at 16 KiB, not every group at every size. That adds about 4 benchmarks (AES-GCM and SHA-256 scalar, plus HMAC later), roughly 4 × 0.7 s × 6 runs ≈ 17 s, and only on PRs whose changed modules select them (Benchmarks: run only the benchmarks of the modules a change touches #133's USES). The full cross-product (every group, size and backend) goes to step 5.
3. Report throughput. Read throughput from criterion's benchmark.json and add MiB/s columns for Base and Head. That costs no benchmark time.
4. Say which backend ran. Add a line to the summary listing the CPU features the runner detected (cpu::detected), so a reviewer can see why a number is what it is. This is what would have caught GHASH on x86-64: carry-less products with mul (BearSSL ctmul64) #142 at a glance.
5. Put the slow sweep on a schedule, not on PRs. A weekly (or workflow_dispatch) run of every benchmark on every backend, at every size plus one large size (e.g. 1 MiB) and a keyed-once variant that moves setup out of the loop. It compares with the previous week's commit through the existing base_commit input. This is where steady-state throughput and overall trends live, and it adds nothing to per-PR time.
6. Optional: like-for-like OpenSSL rows. For scalar rows, run OpenSSL's reference with OPENSSL_ia32cap masking AES-NI and PCLMULQDQ, so "Head vs OpenSSL" compares against OpenSSL's own portable code rather than its AES-NI code. That's one extra OpenSSL run per affected group, and it can be dropped if it isn't worth the time.

Backends that need features no hosted runner has (e.g. future AVX-512/VAES) can't be timed meaningfully under SDE. Those rows should show "not benchmarked (runner lacks X)" instead of disappearing.

Order: 1 to 4 in one PR (bench crate and ci/bench_compare.py only, no library or Lean changes). 5 follows in its own PR, and 6 only if wanted. Once the first PR lands, #142's Benchmarks run should show the scalar speedup.

🤖 Generated with Claude Code

[alex]

I think this is likely all the wrong abstractions. I think we want something like:

• Env var to override CPU feature detection
• Matrix entry for everything we care about
• Remove any custom internal APIs like __with_features

The way to get proper coverage is to actually run stuff end to end under different configurations, not to attempt to special case everything.

[reaperhulk]

Agreed. Running everything end to end under each configuration is simpler, and it tests and benchmarks exactly what a user on that CPU gets. It also means one mechanism instead of a mask threaded through every API. Here is a revised plan that replaces steps 1, 2 and 6 above.

A correction first: SHA-256 isn't actually affected. bench/benches/primitives/sha256.rs already has a sha256-baseline group using __with_features(0). That special case is exactly the kind of thing this plan would remove.

Revised plan

1. VG_CPU_FEATURES overrides detection. It holds a list of feature names (none, or e.g. aes,pclmulqdq), and cpu::detected() returns the detected features intersected with that list. It can only remove features, never add them, so it can't select code the CPU can't run. That keeps the existing safety argument in src/cpu.rs. An unknown name is a build error, so a typo can't silently test the wrong thing.
   • The crate is no_std, and CLAUDE.md forbids hand-written extern blocks, so it can't call getenv. The simplest option is to read it at build time with option_env!. Cargo already tracks that and rebuilds when it changes, and a normal build never has it set.
   • A runtime variable, like OpenSSL's OPENSSL_ia32cap, would need a new std feature. I'd start with the build-time version unless someone wants to flip it on an installed binary.
2. Remove __with_features from AesGcm, AesGcmStream, the hash macro and Hmac, along with the mask loops in tests/cavp/, tests/wycheproof/ and tests/rfc2202/, and the sha256-baseline and scalar HMAC bench groups. cpu.rs's own unit tests keep testing masking directly.
3. A CI matrix entry per configuration. On x86-64 that's all (today's rust-sde job, native or under SDE) plus none (the baseline ISA, native). New configurations are added as backends appear (e.g. AVX2 without AES-NI). Merged coverage across entries covers every backend's Rust dispatch lines. To keep wall time where it is, the feature entries run in parallel with the existing jobs and only run the tests of modules that dispatch, as rust-sde does today.
4. The same matrix for benchmarks. ci/bench_arches.py emits a (arch, VG_CPU_FEATURES) entry for each configuration that one of the changed modules dispatches on. GHASH on x86-64: carry-less products with mul (BearSSL ctmul64) #142 would get x86_64 plus x86_64 / none, and the second one would show the scalar GHASH speedup. They run in parallel, so per-PR wall time doesn't change, and only PRs that touch a dispatching module pay the extra runner minutes. Benchmark ids stay as they are.
5. Unchanged from the original plan: MiB/s columns, the configuration and detected features in each job's summary, and the weekly/manual full sweep (1 MiB, and key setup outside the loop).

Order: 1 to 3 in one PR (the library, tests and ci.yml), then 4 and 5. None of this touches Lean, TCB/ or Spec/.

---

Generated by Claude Code

[alex]

I think an env var that's only available with a build feature seems fine.

…

On Mon, Sep 28, 2026 at 11:35 PM Paul Kehrer ***@***.***> wrote: *reaperhulk* left a comment (pyca/verified-garbage#161) <#161 (comment)> Agreed. Running everything end to end under each configuration is simpler, and it tests and benchmarks exactly what a user on that CPU gets. It also means one mechanism instead of a mask threaded through every API. Here is a revised plan that replaces steps 1, 2 and 6 above. A correction first: SHA-256 isn't actually affected. bench/benches/primitives/sha256.rs already has a sha256-baseline group using __with_features(0). That special case is exactly the kind of thing this plan would remove. Revised plan 1. *VG_CPU_FEATURES overrides detection.* It holds a list of feature names (none, or e.g. aes,pclmulqdq), and cpu::detected() returns the detected features intersected with that list. It can only remove features, never add them, so it can't select code the CPU can't run. That keeps the existing safety argument in src/cpu.rs. An unknown name is a build error, so a typo can't silently test the wrong thing. - The crate is no_std, and CLAUDE.md forbids hand-written extern blocks, so it can't call getenv. The simplest option is to read it at build time with option_env!. Cargo already tracks that and rebuilds when it changes, and a normal build never has it set. - A runtime variable, like OpenSSL's OPENSSL_ia32cap, would need a new std feature. I'd start with the build-time version unless someone wants to flip it on an installed binary. 2. *Remove __with_features* from AesGcm, AesGcmStream, the hash macro and Hmac, along with the mask loops in tests/cavp/, tests/wycheproof/ and tests/rfc2202/, and the sha256-baseline and scalar HMAC bench groups. cpu.rs's own unit tests keep testing masking directly. 3. *A CI matrix entry per configuration.* On x86-64 that's all (today's rust-sde job, native or under SDE) plus none (the baseline ISA, native). New configurations are added as backends appear (e.g. AVX2 without AES-NI). Merged coverage across entries covers every backend's Rust dispatch lines. To keep wall time where it is, the feature entries run in parallel with the existing jobs and only run the tests of modules that dispatch, as rust-sde does today. 4. *The same matrix for benchmarks.* ci/bench_arches.py emits a (arch, VG_CPU_FEATURES) entry for each configuration that one of the changed modules dispatches on. #142 <#142> would get x86_64 plus x86_64 / none, and the second one would show the scalar GHASH speedup. They run in parallel, so per-PR wall time doesn't change, and only PRs that touch a dispatching module pay the extra runner minutes. Benchmark ids stay as they are. 5. Unchanged from the original plan: MiB/s columns, the configuration and detected features in each job's summary, and the weekly/manual full sweep (1 MiB, and key setup outside the loop). Order: 1 to 3 in one PR (the library, tests and ci.yml), then 4 and 5. None of this touches Lean, TCB/ or Spec/. ------------------------------ *Generated by Claude Code <https://claude.ai/code>* — Reply to this email directly, view it on GitHub <#161?email_source=notifications&email_token=AAAAGBDCDRSMWNVYHQF3MMD5RMUXVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBYGMYTEMRQGUZKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5883122052>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBCVUT6TGSQFYLMBOZ35RMUXVAVCNFSNUABGKJSXA33TNF2G64TZHMYTGOBZHEZTCMBRGI5US43TOVSTWNJWGIYDCNBXGA4TDILWAI> . Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS <https://github.com/notifications/mobile/ios/AAAAGBFM2OPCHK4BC237RZ35RMUXVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBYGMYTEMRQGUZKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJKTGN5XXIZLSL5UW64Y> and Android <https://github.com/notifications/mobile/android/AAAAGBGRJWHPR4RXS7TWUDT5RMUXVA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKOBYGMYTEMRQGUZKM4TFMFZW63VHMNXW23LFNZ2KKZLWMVXHJLTGN5XXIZLSL5QW4ZDSN5UWI>. Download it today! You are receiving this because you commented.Message ID: <pyca/verified-garbage/issues/161/5883122052 ***@***.*** com>

-- All that is necessary for evil to succeed is for good people to do nothing.