TD-T22: close as investigated, no gap — the AVX2 int polyfills are not scalar - #262
Conversation
…t scalar
The ticket asked a question ("Investigation only — needs simd_avx2.rs read
first"; the lowering was marked "still optional"). This answers it with an
asm artifact instead of a lowering. Docs only: no code, no new `unsafe`.
Finding: the `avx2_int_type!` polyfills are scalar in the SOURCE and packed
AVX2 in the BINARY. `.cargo/config.toml` pins `-Ctarget-cpu=x86-64-v3` for
every x86_64 build, so LLVM auto-vectorizes the macro's loop bodies.
Measured on `crate::simd::U32x16` (confirmed macro-generated `[u32; 16]`
storage, simd_avx2.rs:1542), release, per-symbol instruction histograms:
arx_quarter_round 2 vpaddd, 2 vpxor, 2 vpshufb 0 scalar ALU
arx_ten_rounds 8 vpaddd, 8 vpxor, 6 vpshufb,
4 vpsrld/vpslld/vpor, 1 jne 0 scalar ALU
reduce_sum 4 vpaddd, 2 vpshufd,
1 vextracti128, 1 vmovd 0 scalar ALU
Three things worth naming:
- `rotate_left(16)` is strength-reduced to a BYTE SHUFFLE (`vpshufb`) —
cheaper than the shl|shr|or triple a hand-written intrinsic emits.
- The ARX loop hits the instruction floor: 4 `U32x16` adds = 64 u32 lanes,
which on 256-bit AVX2 needs 8 ymm adds minimum, and exactly 8 `vpaddd`
are emitted. No headroom exists to recover.
- `reduce_sum` compiles to a logarithmic horizontal-reduction tree, not the
scalar `wrapping_add` fold its source literally spells out.
The float side matches: `F32x16::mul_add` has the same `to_array` → scalar
loop → `from_array` shape and `add_mul_f32` built on it emits real
`vfmadd213ps` — one rounding, mantissa preserved. `array_chunks` /
`array_windows` (+ `_checked`) already exist as the slice-level primitives.
Consequences recorded:
- The `🟠 scalar polyfill` cells and the `⏳` marker are ACCURATE AS WRITTEN
and describe a source-level property. They are not a performance defect.
- A lowering can only be justified by `repr(align(64))` cacheline
guarantees (which the polyfill HAS and a `repr(transparent)` `__m256i`
wrapper LOSES — measured: U32x8 size 64→32, U32x16 align 64→32),
non-inlined ABI shape, or opt-level/LLVM-version independence. Never
by speed.
- `U32x8` must not be introduced as `U32x16`'s building block (operator
ruling): it splits the lane vocabulary for no gain.
- The guard that was missing is a codegen/bench oracle, not a parity
harness — x86_64 is the CI host and `simd.rs`'s
`u32x16_arx_ops_match_scalar` already gates the ARX triple cross-backend.
Ledger entries annotated append-only; the original text is unchanged.
Artifact with probe source, commands, and full histograms:
`.claude/knowledge/td-t22-asm-investigation.md`.
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTD-T22 documentation now records that scalar-looking AVX2 integer polyfills produce packed AVX2 instructions under the configured target baseline. The investigation includes probe methodology and measured instruction counts, while related matrix and audit entries are marked closed. ChangesTD-T22 closure
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_35b29635-2e59-43e8-9f4b-9ff80b0367ac) |
…ured Self-caught overclaim in the previous commit. I wrote "0 scalar ALU" from a narrow grep for `(add|sub|xor|rol|ror|shl|shr|or|and)[lqwb]`. A broad sweep of ALL non-vector instructions across the three probe symbols returns: 3 retq 1 movl 1 jne 1 decl The `decl` IS a scalar ALU instruction — it decrements the `arx_ten_rounds` loop trip count. So "zero scalar ALU instructions" is literally false. The claim that survives, and the one the finding actually rests on: **no scalar operation touches lane data.** Every u32 lane op in all three probes is a packed AVX2 instruction; the only scalar work is loop control. Corrected in both sites and stated explicitly in the artifact, since a knowledge doc that overstates its own measurement is the failure mode this investigation exists to document.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5bd1e4b038
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| **No gap. The "scalar polyfill" is not scalar in the emitted binary.** | ||
| `.cargo/config.toml` pins `-Ctarget-cpu=x86-64-v3` (AVX2+FMA) for **every** | ||
| x86_64 build including CI, so LLVM auto-vectorizes the macro's | ||
| `for i in 0..N { … }` bodies into packed AVX2. Measured below: **zero scalar | ||
| ALU instructions**, and the ARX loop hits the theoretical instruction minimum. |
There was a problem hiding this comment.
The x86-64-v3 rustflag only enables the AVX2 instruction set; it does not enable LLVM's loop vectorizer. The workspace has no optimized default dev/test profile, so ordinary cargo build and CI cargo test compile these loops at low optimization and can retain scalar ALU—the note itself concedes at lines 146-147 that auto-vectorization does not hold at -O0/-O1. Claiming this applies to every x86_64 build therefore overstates the evidence from the single --release probe and can incorrectly close a performance gap for development/test profiles.
Useful? React with 👍 / 👎.
| shuffle** (`vpshufb`) — cheaper than the `shl|shr|or` triple a hand-written | ||
| intrinsic version would emit. | ||
|
|
||
| **`arx_ten_rounds`** — the full ChaCha20 double-round over `[U32x16; 4]`: |
There was a problem hiding this comment.
This label is inaccurate: arx_ten_rounds repeats one quarter-round over four vectors, whereas vendor/chacha20/src/backends/ndarray_simd.rs::ks16 keeps 16 state vectors live and invokes eight distinct quarter-rounds per double-round, followed by feed-forward. That real workload has substantially different register pressure and spills, so the reported eight-add minimum does not establish that the production ChaCha path has no headroom; use the actual ks16 shape before treating this result as the basis for closing TD-T22.
Useful? React with 👍 / 👎.
| > reduction tree for `reduce_sum`. **No `__m256i` lowering is warranted on | ||
| > codegen grounds** for `U16x16`/`U32x8`/`U64x4`/`I32x8`/`I64x4` or the | ||
| > existing wider types. Artifact: |
There was a problem hiding this comment.
At the reviewed revision, src/simd_avx2.rs:1577-1579 already defines U16x16 as a #[repr(transparent)] wrapper around __m256i, introduced by the earlier TD-T22 lowering, rather than through avx2_int_type!. Including it in a conclusion derived from scalar-array loop codegen falsely records the current implementation and may discourage maintenance of an intrinsic path that is already used by the PQ4-ADC kernel.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/knowledge/agnostic-surface-cpu-matrix.md:
- Around line 101-118: Synchronize the closed TD-T22 status across both
knowledge documents: in .claude/knowledge/agnostic-surface-cpu-matrix.md lines
101-118, remove or annotate the remaining ⏳ markers and “Needs verification”
legend; in .claude/knowledge/td-simd-tier-audit.md lines 274-290, update the
retained “Audit pending” entry as superseded. Preserve the documented conclusion
that no performance defect remains.
In @.claude/knowledge/td-t22-asm-investigation.md:
- Around line 104-109: Revise the “instruction-count floor” discussion in the
rolled-loop analysis to limit the proof to the four U32x16 additions producing
exactly eight minimum vpaddd instructions. Remove or narrow the claim that there
is no headroom for handwritten code, since the evidence does not establish that
shuffles, moves, or loop/control instructions cannot be reduced.
- Around line 26-32: Qualify the closure claims to match the measured scope: in
.claude/knowledge/td-t22-asm-investigation.md lines 26-32, identify the
demonstrated result as applying to U32x16 under release x86-64-v3 unless broader
probes are added; in .claude/knowledge/agnostic-surface-cpu-matrix.md lines
101-112, do not close all referenced width cells from that single measurement;
and in .claude/knowledge/td-simd-tier-audit.md lines 274-285, qualify the
no-lowering conclusion or add measurements for every listed width.
- Around line 152-157: Update the CI claims in the discussion of cargo test and
asm-diff/bench checks so they do not assume x86_64 implies AVX2 support. Confirm
the referenced runners are explicitly AVX2-capable, or revise the guidance to
gate or partition native cargo test and asm-diff/bench execution based on actual
AVX2 capability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ce3dd8f-593e-4c72-871f-497fa2f8be28
📒 Files selected for processing (3)
.claude/knowledge/agnostic-surface-cpu-matrix.md.claude/knowledge/td-simd-tier-audit.md.claude/knowledge/td-t22-asm-investigation.md
| > **⏳ RESOLVED — TD-T22 CLOSED, no gap (2026-07-28).** The audit is done and | ||
| > the answer is: the storage IS scalar in the SOURCE, and that costs nothing. | ||
| > `.cargo/config.toml` pins `-Ctarget-cpu=x86-64-v3` for every x86_64 build, | ||
| > so LLVM auto-vectorizes the `avx2_int_type!` loop bodies into packed AVX2. | ||
| > Measured on the ChaCha20 ARX triple over `U32x16`: **zero scalar ALU | ||
| > instructions**, `rotate_left(16)` strength-reduced to `vpshufb`, and the | ||
| > 10-round double-round loop emits exactly **8 `vpaddd` for 64 u32 lanes** — | ||
| > the AVX2 instruction-count floor, with no headroom a hand-written | ||
| > `__m256i` version could recover. `reduce_sum` emits a logarithmic | ||
| > `vpaddd`/`vpshufd`/`vextracti128` reduction tree, not the scalar fold its | ||
| > source spells out. The float side matches: `F32x16::mul_add` is the same | ||
| > `to_array` → loop → `from_array` shape and emits real `vfmadd213ps`. | ||
| > | ||
| > **So these ⏳ cells are ACCURATE AS WRITTEN and must not be read as a | ||
| > performance defect.** A lowering can only be justified by `repr(align(64))` | ||
| > cacheline guarantees (which the polyfill already has and a | ||
| > `repr(transparent)` wrapper would LOSE), non-inlined ABI shape, or | ||
| > `opt-level`/LLVM-version independence — never by speed. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Synchronize the closed status across both knowledge documents. The append-only closure leaves contradictory pending markers in the documentation.
.claude/knowledge/agnostic-surface-cpu-matrix.md#L101-L118: remove or annotate the remaining⏳cells and “Needs verification” legend..claude/knowledge/td-simd-tier-audit.md#L274-L290: update or mark the retained “Audit pending” entry as superseded.
📍 Affects 2 files
.claude/knowledge/agnostic-surface-cpu-matrix.md#L101-L118(this comment).claude/knowledge/td-simd-tier-audit.md#L274-L290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/knowledge/agnostic-surface-cpu-matrix.md around lines 101 - 118,
Synchronize the closed TD-T22 status across both knowledge documents: in
.claude/knowledge/agnostic-surface-cpu-matrix.md lines 101-118, remove or
annotate the remaining ⏳ markers and “Needs verification” legend; in
.claude/knowledge/td-simd-tier-audit.md lines 274-290, update the retained
“Audit pending” entry as superseded. Preserve the documented conclusion that no
performance defect remains.
| ## Answer | ||
|
|
||
| **No gap. The "scalar polyfill" is not scalar in the emitted binary.** | ||
| `.cargo/config.toml` pins `-Ctarget-cpu=x86-64-v3` (AVX2+FMA) for **every** | ||
| x86_64 build including CI, so LLVM auto-vectorizes the macro's | ||
| `for i in 0..N { … }` bodies into packed AVX2. Measured below: **zero scalar | ||
| ALU instructions**, and the ARX loop hits the theoretical instruction minimum. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Align the closure claims with the measured probe scope. The supplied evidence covers U32x16 under the release x86-64-v3 configuration, but the dependent documents resolve several other widths as equivalent.
.claude/knowledge/td-t22-asm-investigation.md#L26-L32: state that the demonstrated result is for the measured type/configuration, or add probes for the broader claim..claude/knowledge/agnostic-surface-cpu-matrix.md#L101-L112: avoid closing all referenced cells based solely on theU32x16measurement..claude/knowledge/td-simd-tier-audit.md#L274-L285: qualify the no-lowering conclusion or provide measurements for each listed width.
📍 Affects 3 files
.claude/knowledge/td-t22-asm-investigation.md#L26-L32(this comment).claude/knowledge/agnostic-surface-cpu-matrix.md#L101-L112.claude/knowledge/td-simd-tier-audit.md#L274-L285
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/knowledge/td-t22-asm-investigation.md around lines 26 - 32, Qualify
the closure claims to match the measured scope: in
.claude/knowledge/td-t22-asm-investigation.md lines 26-32, identify the
demonstrated result as applying to U32x16 under release x86-64-v3 unless broader
probes are added; in .claude/knowledge/agnostic-surface-cpu-matrix.md lines
101-112, do not close all referenced width cells from that single measurement;
and in .claude/knowledge/td-simd-tier-audit.md lines 274-285, qualify the
no-lowering conclusion or add measurements for every listed width.
| 4. **The guard that was missing is a codegen oracle, not a parity harness.** | ||
| x86_64 is the CI host, so `cargo test` already runs the AVX2 tier | ||
| natively and `simd.rs`'s `u32x16_arx_ops_match_scalar` already gates every | ||
| backend's ARX triple against scalar. What did not exist — and what would | ||
| have answered this ticket in twenty minutes — is an **asm-diff / bench** | ||
| check. Adding one to CI is the sanctioned follow-up. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
rg -n --hidden --glob '!target' \
'target-cpu|x86-64-v3|avx2|cargo test|runs-on' \
.cargo .github 2>/dev/null || true
lscpu | rg -i 'Architecture|Flags' || trueRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu
printf 'Tracked CI/config-related paths:\n'
git ls-files | rg '(^\.github/|^\.cargo/|Cargo\.toml$|Cargo\.lock$|^rust-toolchain\.toml$|\.gitlab-ci\.yml$|tox\.ini$|pyproject\.toml$|^README\.md$)' || true
printf '\nSearch targeted terms in small config/text files:\n'
rg -n --hidden --glob '!target' --glob '!*.lock' \
'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo +|RUSTFLAGS|RUST_TEST_THREADS|RUST_BACKTRACE|runs-on:|runner:' \
.cargo .github Cargo.toml rust-toolchain.toml README.md 2>/dev/null || trueRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
printf 'Repository root files:\n'
find . -maxdepth 2 \( -type f -o -type d \) 2>/dev/null | sed 's#^\./##' | sort | head -200
printf '\nCI/config tracked paths:\n'
git ls-files 2>/dev/null | sort | rg '(^\.github/|^\.cargo/|^Cargo\.toml$|^Cargo\.lock$|^rust-toolchain\.toml$|^\.gitlab-ci\.yml$|^tox\.ini$|^pyproject\.toml$|^README\.md$|^scripts/|^build\.rs$)' || true
printf '\nTargeted term matches (config files only):\n'
git ls-files 2>/dev/null | rg '(^\.github/|^\.cargo/|^Cargo\.toml$|^rust-toolchain\.toml$|^README\.md$|^scripts/|^build\.rs$)' | while read -r f; do
echo "--- $f"
rg -n -i 'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo \+|RUSTFLAGS|RUST_TEST_THREADS|RUST_BACKTRACE|runs-on:|runner:|actions/runner' "$f" 2>/dev/null || true
doneRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
printf 'pwd and quick directory count:\n'
pwd
find . -maxdepth 3 -type d -print 2>/dev/null | wc -l
find . -maxdepth 3 \( -type f -o -type d \) 2>/dev/null | head -200
printf '\nTracked CI/config paths:\n'
git ls-files 2>/dev/null | sort | head -500 | rg '(^\.github/|^\.cargo/|^Cargo\.toml$|^rust-toolchain\.toml$|^README\.md$|^scripts/|^build\.rs$)' || true
printf '\nTracked all files count/sample:\n'
git ls-files 2>/dev/null | wc -l
git ls-files 2>/dev/null | head -100Repository: AdaWorldAPI/ndarray
Length of output: 5157
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
printf 'Relevant CI/config files with targeted matches:\n'
for f in $(git ls-files 2>/dev/null | rg '(^\.github/|^\.cargo/config[^/]*\.toml$|^Cargo\.toml$|^rust-toolchain\.toml$|^README\.md$|^scripts/.*\.sh$|\.gitlab-ci\.yml$)' || true); do
p=$(printf '%s' "$f" | sed 's/^\.\///')
echo "--- $p"
rg -n -i 'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo \+|RUSTFLAGS|RUST_TEST_THREADS|RUST_BACKTRACE|runs-on:|runner:|actions/runner|nightly|default-target|default_target|required-features|features=' "$p" 2>/dev/null || true
done
printf '\nCloned known files summaries:\n'
for f in Cargo.toml rust-toolchain.toml README.md .cargo/config.toml .cargo/config-native.toml .cargo/config-apple-m2.toml .cargo/config-pi5.toml .cargo/config-graviton.toml .cargo/config-avx512.toml scripts/all-tests.sh; do
if [ -f "$f" ]; then
echo "===== $f ====="
cat -n "$f" | sed -n '1,220p'
fi
doneRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
printf 'Cargo workspace configs:\n'
sed -n '1,180p' Cargo.toml || true
printf '\nRust toolchain:\n'
cat rust-toolchain.toml || true
printf '\nCI config files present:\n'
find .github .cargo scripts README.md Cargo.toml rust-toolchain.toml 2>/dev/null -type f | sort
printf '\nGitHub config snippets:\n'
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] && { echo "===== $f ====="; rg -n -i 'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo \+|RUSTFLAGS|runs-on:|runner:|actions/runner|nightly|default-target|features=' "$f" || true; }
done
printf '\nCargo config snippets:\n'
for f in .cargo/config.toml .cargo/config-native.toml .cargo/config-apple-m2.toml .cargo/config-pi5.toml .cargo/config-graviton.toml .cargo/config-avx512.toml .cargo/config-wasm.toml; do
[ -f "$f" ] && { echo "===== $f ====="; sed -n '1,200p' "$f"; }
doneRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
printf 'Quick extract config snippets to temp:\n'
for f in Cargo.toml rust-toolchain.toml README.md .cargo/config.toml .cargo/config-native.toml .cargo/config-apple-m2.toml .cargo/config-pi5.toml .cargo/config-graviton.toml .cargo/config-avx512.toml .cargo/config-wasm.toml; do
if [ -f "$f" ]; then
echo "===== $f ====="
sed -n '1,220p' "$f" > "$tmpdir/$f" || true
rg -n -i 'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo \+|RUSTFLAGS|RUST_TEST_THREADS|RUST_BACKTRACE|runs-on:|runner:|actions/runner|nightly|default-target|features=' "$tmpdir/$f" || true
fi
done
echo "===== GitHub workflows quick scan ====="
for f in .github/workflows/*.yml .github/workflows/*.yaml; do
[ -f "$f" ] && { echo "===== $f ====="; sed -n '1,220p' "$f"; }
done
printf '\nNative AVX512 config existence/key lines:\n'
wc -l .cargo/config-avx512.toml .cargo/config-native.toml 2>/dev/null || true
rg -n -i 'avx512|x86-64-v3|target-cpu|cargo' .cargo/config-avx512.toml .cargo/config-native.toml 2>/dev/null || trueRepository: AdaWorldAPI/ndarray
Length of output: 149
🏁 Script executed:
#!/bin/bash
set -eu -o pipefail
tmpdir=$(mktemp -d)
trap 'rm -rf "$tmpdir"' EXIT
printf 'Copying known config files before grep:\n'
for f in Cargo.toml rust-toolchain.toml README.md .cargo/config.toml .cargo/config-native.toml .cargo/config-apple-m2.toml .cargo/config-pi5.toml .cargo/config-graviton.toml .cargo/config-avx512.toml .cargo/config-wasm.toml; do
if [ -f "$f" ]; then
sed -p '1,220p' "$f" > "$tmpdir/$f" || true
echo "copied $f"
else
echo "missing $f"
fi
done
printf '\nMatches from temp copies:\n'
for f in "$tmpdir"/Cargo.toml "$tmpdir"/rust-toolchain.toml "$tmpdir"/README.md "$tmpdir"/.cargo/* "$tmpdir"/.github/workflows/*; do
if [ -f "$f" ]; then
echo "--- $f"
rg -n -i 'target-cpu|x86-64-v3|x86_64|avx2|cargo test|cargo \+|RUSTFLAGS|RUST_TEST_THREADS|RUST_BACKTRACE|runs-on:|runner:|actions/runner|nightly|default-target|features=' "$f" || true
fi
doneRepository: AdaWorldAPI/ndarray
Length of output: 7485
Do not equate x86_64 with AVX2 runtime support.
--cfg x86_64 only indicates the build target, not that the CI runner supports AVX2. Confirm the CI runners used by .claude/knowledge/td-t22-asm-investigation.md are explicitly AVX2-capable, or gate/partition the native cargo test and asm-diff/bench checks accordingly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/knowledge/td-t22-asm-investigation.md around lines 152 - 157, Update
the CI claims in the discussion of cargo test and asm-diff/bench checks so they
do not assume x86_64 implies AVX2 support. Confirm the referenced runners are
explicitly AVX2-capable, or revise the guidance to gate or partition native
cargo test and asm-diff/bench execution based on actual AVX2 capability.
2 participants
The deliverable TD-T22 actually asked for. Docs only — no code, no new
unsafe.td-simd-tier-audit.md:339readsTD-T22 | – | – | Investigation only, andCHACHA20_MATRYOSHKA_PLAN.md:104calls the lowering "still optional". This answers the question with an assembly artifact instead of performing the optional lowering.Finding
The
avx2_int_type!polyfills are scalar in the SOURCE and packed AVX2 in the BINARY..cargo/config.tomlpins-Ctarget-cpu=x86-64-v3for every x86_64 build including CI, so LLVM auto-vectorizes the macro'sfor i in 0..Nbodies.Measured on
crate::simd::U32x16— confirmed macro-generated[u32; 16]storage atsimd_avx2.rs:1542, no hand-written struct — release build, per-symbol instruction histograms:arx_quarter_roundvpaddd, 2vpxor, 2vpshufbarx_ten_roundsvpaddd, 8vpxor, 6vpshufb, 4×vpsrld/vpslld/vpor, 1jnereduce_sumvpaddd, 2vpshufd, 1vextracti128, 1vmovdThree details worth naming:
rotate_left(16)is strength-reduced to a byte shuffle (vpshufb) — cheaper than theshl|shr|ortriple a hand-written intrinsic version emits.U32x16adds = 64 u32 lanes; on 256-bit AVX2 that needs 8 ymm adds minimum, and exactly 8vpadddare emitted. There is no headroom for a hand-written version to recover.reduce_sumcompiles to a logarithmic reduction tree, not the scalarwrapping_addfold its source literally spells out.The float side matches:
F32x16::mul_addhas the identicalto_array→ scalar loop →from_arrayshape, andadd_mul_f32built on it emits realvfmadd213ps— one rounding, mantissa preserved.array_chunks/array_windows(+_checked) already exist as the slice-level primitives.What this changes
🟠 scalar polyfillcells and the⏳marker are accurate as written. They describe a source-level property and must not be read as a performance defect. Both ledger sites now say so.__m256ilowering is warranted on codegen grounds — forU16x16,U32x8,U64x4,I32x8,I64x4, or the wider types.repr(align(64))cacheline guarantees (which the polyfill has and arepr(transparent)__m256iwrapper loses — measured:U32x8size 64→32,U32x16align 64→32), non-inlined ABI shape, oropt-level/LLVM-version independence. Never by speed.U32x8must not becomeU32x16's building block (operator ruling) — it splits the lane vocabulary for no gain.cargo testalready runs the AVX2 tier natively, andsimd.rs'su32x16_arx_ops_match_scalaralready gates the ARX triple across every backend. An asm-diff or bench check in CI is the sanctioned follow-up — offered, not built, since it's separate scope.Files
.claude/knowledge/td-t22-asm-investigation.md(new) — the artifact: probe source, exact commands, per-symbol histograms, and the reasoning..claude/knowledge/agnostic-surface-cpu-matrix.md—⏳resolved, appended below the original marker, which is unchanged..claude/knowledge/td-simd-tier-audit.md— TD-T22 marked closed, appended above the original entry, which is unchanged.Ledger text is annotated append-only; the sole deletion in the diff is the section header being re-titled with its closure status.
Provenance
This investigation follows a lowering PR (#261, closed unmerged) that shipped ~700 lines of hand-written intrinsics on the premise these polyfills executed scalar code. They don't. That PR was closed on the
U32x8-composition ruling; this is the work the ticket originally asked for.Generated by Claude Code
Summary by CodeRabbit