test-coverage: src/bits/popcount.rs flaps ±26pp on x86 because the AVX-512 VPOPCNTDQ path is CPU-conditional on the runner

[newhoggy]

Category: test-coverage / SIMD dispatch — surfaced by the coverage comment on PR #287 (a one-line dorny/paths-filter dependabot bump that changes no Rust code).

Problem

src/bits/popcount.rs swings ±26pp on the x86_64 coverage leg between otherwise-unrelated PRs, because its AVX-512 VPOPCNTDQ path is only executed on runners whose CPU actually has the instruction. The x86 SIMD gate (.github/workflows/ci.yml:76) pins SUCCINCTLY_EXPECT_SIMD: sse2,sse4.2,avx2,bmi2,popcnt — avx512vpopcntdq is deliberately not required (GitHub-hosted ubuntu-latest doesn't guarantee it). So on the coverage job (also ubuntu-latest), the AVX-512 region is present-but-uncalled on some hosts and executed on others.

The CPU-gated lines, all compiled under --features cli,simd,regex,serde but only reachable with avx512vpopcntdq:

• popcount_words_avx512vpopcntdq body — popcount.rs:218-245 (loop, _mm512_loadu_si512, _mm512_popcnt_epi64, _mm512_reduce_add_epi64, scalar tail).
• The runtime-dispatch arm in popcount_words_x86 — popcount.rs:261-266 (if is_x86_feature_detected!("avx512vpopcntdq") { return unsafe { … } }).
• Both host-gated tests, which early-return on non-supporting CPUs and so neither cover nor validate the kernel there:
  • test_avx512_vpopcntdq_matches_scalar — guard at popcount.rs:407-410.
  • test_avx512_edge_cases — guard at popcount.rs:444-446.

That's ~30 executable lines counted in the file's denominator but coverable only on AVX-512-VPOPCNTDQ hardware.

Evidence

PR #287's coverage-x86_64 comment flagged src/bits/popcount.rs 92.86% → 66.43% (🔴 -26.43pp) under "1 unchanged file(s) also moved (not attributed to this PR)". The diff is a single line in ci.yml — the swing is purely that the main baseline run and the PR-head run drew different runner CPUs (one with the VPOPCNTDQ instruction, one without).

Why this isn't already covered / caught

Unlike the SSE2 (#247) and DSV-dispatch (#283) cases, this path cannot be forced on a CPU that lacks the instruction: _mm512_popcnt_epi64 will SIGILL there, and a software shim would no longer be testing the intrinsic. So the AVX-512 kernel's correctness is inherently only verifiable on AVX-512 hardware, and its coverage is inherently non-deterministic across GitHub's runner fleet.

Practical severity: low (cosmetic)

• The coverage bot already quarantines this into "unchanged files … not attributed to this PR" and does not count it as a patch-coverage regression.
• The fail-under-lines: 55 gate (ci.yml:523) has ~19pp of headroom over the ~74.5% total, so a one-file -26pp wobble won't trip it.

So this is recurring visual noise in the sticky comment, not a gate or correctness problem. Filing it so the noise is understood and (optionally) silenced deterministically, rather than re-diagnosed on every unrelated PR.

Options (pick one)

1. Exclude the hardware-gated region from coverage measurement so popcount.rs reports a stable number regardless of runner CPU. Depends on what action-works/omni-dev-coverage-check / cargo-llvm-cov expose — nightly #[coverage(off)] on popcount_words_avx512vpopcntdq + the dispatch arm + the two host-gated tests, or --ignore-filename-regex for the whole file, or an omni-dev ignore-list config. (Preferred if a region-level mechanism exists; whole-file exclusion is the blunt fallback.)
2. Teach omni-dev coverage diff to suppress a configured allowlist of known-CPU-conditional files (src/bits/popcount.rs, and arguably the src/dsv/simd/* and src/yaml/simd/* dispatchers) from the "unchanged files also moved" section, so they never surface as noise on unrelated PRs. (Upstream change in the action/omni-dev, not this repo.)
3. Pin the coverage runner to a CPU class that has AVX-512 VPOPCNTDQ (deterministic-up), or add avx512vpopcntdq to a dedicated expect-simd leg. Fragile — GitHub-hosted runner CPUs aren't contractually stable — and it would then require the feature, which contradicts the current intentional non-requirement.
4. Accept the noise, document it. Add a short note (CONTRIBUTING.md "SIMD CI coverage" section, near the existing expect-simd guidance) explaining that popcount.rs flaps by design, so it isn't re-triaged each time. Cheapest; no determinism gain.

Definition of done

• src/bits/popcount.rs's x86 coverage number is deterministic across runner CPUs or the flap is explicitly documented so it's not re-investigated per PR.
• No change to production popcount dispatch or the intentional non-requirement of avx512vpopcntdq.
• If code/config is touched: clippy --all-targets --all-features -D warnings clean; conventional commit with the appropriate scope.

Related

• Trigger: coverage comment on PR chore(ci): bump dorny/paths-filter from 3 to 4 #287 (dorny/paths-filter v3→v4).
• Same family: DSV SIMD dispatcher fallback arms are never exercised end-to-end #283 (DSV SIMD dispatcher fallback arms never exercised), test(ci): CI-executable regression guard for the x86 SSE2 classify/skip-width path (non-AVX2 fallback) #247 (x86 SSE2 classify/skip path has no CI-executable guard).
• Companion mechanism: SUCCINCTLY_EXPECT_SIMD (tests/simd_expectation_tests.rs) pins required features but intentionally omits avx512vpopcntdq.

[newhoggy]

Verification + recommendation

Checked the body's claims against the code and toolchains before picking an option. The citations all hold (kernel + scalar tail, the is_x86_feature_detected!("avx512vpopcntdq") dispatch arm, both host-gated test guards, the intentional avx512vpopcntdq omission from SUCCINCTLY_EXPECT_SIMD, and the ~19pp headroom over fail-under-lines: 55). Two toolchain findings narrow the options:

Option 1's region-level form is not available on the current setup

• #[coverage(off)] is still experimental on stable — verified empirically: bare use fails with E0658 on rustc 1.97.0. The stabilization (Stabilize #[coverage] attribute rust-lang/rust#130766, slated for 1.85) was reverted (Revert stabilization of the #[coverage(..)] attribute rust-lang/rust#134672); re-stabilization is tracked in Tracking issue for stabilizing the #[coverage(..)] attribute rust-lang/rust#134749. The issue body's "nightly" qualifier is exactly right.
• #[cfg_attr(coverage, coverage(off))] doesn't dodge it: cargo llvm-cov does inject --cfg=coverage (verified via cargo llvm-cov show-env), but when the cfg is set on a stable toolchain the expanded attribute still hard-errors (verified: E0658 under rustc --cfg coverage). The only working source-level pattern is the tokio one — crate-root #![cfg_attr(coverage_nightly, feature(coverage_attribute))] plus per-item #[cfg_attr(coverage_nightly, coverage(off))] — which requires collecting coverage on nightly.
• The coverage job installs no toolchain of its own (ci.yml coverage: job → straight into omni-dev-coverage-check), so collection runs on the runner's stable. Moving coverage to nightly just to unlock the attribute is disproportionate for cosmetic noise — and ci.yml's own comment calls nightly the flakiest leg.

Recommendation (ranked)

1. Do Option 4 now — the short "SIMD CI coverage" note. Zero risk, and it removes the actual recurring cost (re-triage on every unrelated PR). Worth doing regardless of what else lands.
2. Pursue Option 2 upstream as the durable fix — a configured allowlist of known CPU-conditional files (src/bits/popcount.rs, arguably the src/dsv/simd/* / src/yaml/simd/* dispatchers) suppressed from the "unchanged files also moved" section. Unlike any exclusion approach it keeps measuring the file — the AVX-512 kernel's coverage on capable runners is real signal worth retaining — while making the sticky-comment noise deterministically gone. Since the action is action-works/omni-dev-coverage-check wrapping omni-dev, the coordination cost should be low.
3. If in-repo determinism is wanted sooner: file-level --ignore-filename-regex 'bits/popcount\.rs' appended to the action's test-args (the --workspace/--features args already pass through to cargo llvm-cov, so this should too — verify on a test PR). Blunt: it hides all of popcount.rs including the ~93% that's legitimately covered, and the CLAUDE.md local-reproduction command must gain the same flag or local vs CI totals will drift.
4. Skip Option 3 (runner pinning) — agree with the body's own assessment; GitHub's runner CPUs aren't contractually stable, and requiring the feature contradicts the intentional non-requirement.

None of 1–3 touch production dispatch, and the two host-gated tests keep running (and validating the kernel) on capable hardware in every case — consistent with the definition of done.