Cpu optimized/q4k sdot gemm -> downstream - #34
Conversation
2d9ade9 to
468bf99
Compare
Adds the int8 SDOT-based quantized GEMM path used for prefill and decode on aarch64 (NEON dotprod) and x86 (AVX2): - Row-tiled GEMM in k_quants::matmul: tile activation rows so each weight column streams once per tile, with rayon-parallel columns; the per-row activation quantization is parallelized over the pool. - GgmlType::vec_dot_multi trait hook + a multi-row NEON kernel that unpacks each Q4_K weight superblock once and dots it against R activation rows, amortizing the nibble-unpack. Bit-identical to the single-row vec_dot. - QStorage/QTensor::cat_rows for bit-exact row concatenation (used by fused projection pairs), plus a QStorage owned-data roundtrip test. All 38 quantized_tests pass (bit-exact vs the scalar reference).
aa9fd26 to
c5d8b64
Compare
Add vdotq_s32_acc (accumulate-chain SDOT) and use it in both vec_dot_q4k_q8k (decode GEMV) and vec_dot_q4k_q8k_xr (multi-row prefill): both sub-block-half dots accumulate into one register via SDOT's native accumulate, then one reduction - instead of vdupq_n_s32(0) + a vaddq per pair. Integer accumulation is associative, so this is bit-identical to the prior form (quantized_tests q4k pass). It brings candle's Q4_K kernels to instruction parity with ggml's hand-tuned C: ~+9.7% cumulative 1-thread decode and ~+4-5% prefill on Graviton2/N1.
When m=1 and the decode pool has a single thread (the Lambda 1-vCPU tier), run the GEMV serially instead of pool.install + into_par_iter - that path is pure rayon split/join overhead per call (decode issues ~200 matmuls/token) with no parallelism to gain. Bit-identical (same per-column vec_dot); gated CANDLE_DECODE_SERIAL (default on, =0 restores the rayon path). ~+2.5% N1 1-thread decode.
|
@codex review |
![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: 635acacc3b
ℹ️ 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".
| pool.install(|| { | ||
| if m == 1 { | ||
| let lhs_row = &lhs_b[..k_in_blocks]; | ||
| dst.into_par_iter() |
There was a problem hiding this comment.
When m == 1 and a caller provides a dst slice longer than m*n, this decode path iterates the entire slice even though the new guard only rejects shorter buffers. For col_idx >= n it reads past the advertised rhs_t columns and overwrites entries outside the output; the previous row-sliced implementation only touched dst[..n]. Slice dst to m*n/n or require exact length before both decode loops.
Useful? React with 👍 / 👎.
… deduplicated pool builders (simplification/reuse)
vec_dot_multi was fixed to vec_dot_q4k_q8k_xr::<2> (the Cortex-X925 tuning, where R=4 spilled). On Neoverse-N1 a weight column needs only 4 nibble vectors, so wider row groups fit the register file and amortize the unpack further. Replace the fixed-R=2 loop with a descending 8->4->2->1 ladder capped by CANDLE_Q4K_XR_R (clamped 2..=8, default 4 = the validated N1 win), and raise the _xr kernel's group cap from 4 to 8. Bit-identical to vec_dot (quantized_tests q4k pass at R=2/4/8). ~+5-9% prefill on N1; the env knob keeps per-arch tuning available.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12e4d796e5
ℹ️ 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".
| // than both 2 (bandwidth-starved) and cores/2 (preemption) on M-series. | ||
| build_qmatmul_pool( | ||
| "CANDLE_QMATMUL_DECODE_THREADS", | ||
| qmatmul_half_cores().saturating_sub(1).max(2), |
There was a problem hiding this comment.
On hosts where available_parallelism() is 1, qmatmul_half_cores() returns 0 but this .max(2) still builds a two-worker decode pool, so the m == 1 && pool.current_num_threads() <= 1 serial fast path below is never used by default. That leaves the common 1-vCPU/Lambda decode case paying Rayon scheduling and oversubscription on every per-token GEMV despite the new serial path being intended to avoid exactly that overhead.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
CPU quantized matmul: SDOT prefill, row-tiled GEMM, multi-row kernel
Adds the int8 SDOT-based quantized GEMM path used for prefill and decode on aarch64 (NEON dotprod) and x86 (AVX2):
Row-tiled GEMM in k_quants::matmul: tile activation rows so each weight column streams once per tile, with rayon-parallel columns; the per-row activation quantization is parallelized over the pool.
GgmlType::vec_dot_multi trait hook + a multi-row NEON kernel that unpacks each Q4_K weight superblock once and dots it against R activation rows, amortizing the nibble-unpack. Bit-identical to the single-row vec_dot.
QStorage/QTensor::cat_rows for bit-exact row concatenation (used by fused projection pairs), plus a QStorage owned-data roundtrip test.