Skip to content

cuda: TurboQuant TQ4_1S decode optimisations (TQ-only half of #338) - #342

Merged
TheTom merged 7 commits into
TheTom:feature/turboquant-kv-cachefrom
jasstrong:pr/tq4_1s-decode-tq
Sep 3, 2026
Merged

cuda: TurboQuant TQ4_1S decode optimisations (TQ-only half of #338)#342
TheTom merged 7 commits into
TheTom:feature/turboquant-kv-cachefrom
jasstrong:pr/tq4_1s-decode-tq

Conversation

@jasstrong

Copy link
Copy Markdown

This is the TQ-only half of #338, split as requested in review; the generic fusions are in the companion PR, which is stacked on this one. Based on the branch tip after #336 merged.

Overview

Four patches on the TurboQuant decode path, all TQ4_1S-specific and all exercised by test-backend-ops:

# (in #338) patch effect
1 enable the int8 dp4a decode path on CDNA 94.3 → 95.4 t/s
5 cache the TQ activation pre-rotation across MoE projections ~neutral, structural
6 decode TQ4 centroids with v_perm_b32 instead of shift arithmetic +14%
8 lanes-per-row mapping for the TQ4_1S MoE decode matvec +3.1%

Patch 2 of #338 (the deep TQ4_1S MoE down-proj + weighted-sum fuse) is dropped entirely: the generic reduce tail in the companion PR measured faster than it (100.0 vs 99.0 tg128), so there is no reason to carry the slower kernel behind a knob.

Changes relative to #338 from the review:

  • Patch 5 now carries the per-eval epoch increment it relied on (in cuda: TurboQuant TQ4_1S decode optimisations #338 that line lived in patch 3), and no longer touches the dropped deep-fuse kernel.
  • Patch 8's lanes-per-row default of 16 is gated to AMD (GGML_CUDA_CC_IS_AMD(cc) ? 16 : 32), since every measurement behind it is MI210; NVIDIA keeps 32.
  • The duplicated "16 measured best on CDNA" comment is gone.
  • Tom's nvcc/MUSA guard for tq4_cents8_reg() (949de16) is carried over.

Additional information

MI210 (gfx90a), ROCm 7.2.3, Qwen3.6-35B-A3B-TQ4max: numbers per patch are as measured in #338; the combined figures for this subset are in the first comment below once CI is green.

test-backend-ops -o MUL_MAT -p type_a=tq4_1s and -o MUL_MAT_ID: 149/149 and 38/38 on the MI210 (gfx90a, ROCm 7.2.3), with GGML_TQ_MMQ=1 set so the native path is the one exercised. The full -o MUL_MAT suite is 1697/1697 on the final run of this branch; an earlier run showed the q5_cr ConvRot case flaking, as it does on the pristine base on random data.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES — Claude Code was used throughout this series: profiling and diagnosis, the kernel work, commit messages, the review fixes and the verification runs.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NxP6x5bmUDYFvmouceN2mR

jas and others added 5 commits September 3, 2026 04:33
The MoE decode dispatch gated the int8 dp4a path behind
!GGML_CUDA_CC_IS_AMD, which excluded CDNA even though gfx90a has the
dot4 instructions. Enable it there, keeping RDNA on the scalar path.

Decode 94.3 -> 95.4 t/s on an MI210 with Qwen3.6-35B-A3B-TQ4max.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NxP6x5bmUDYFvmouceN2mR
(cherry picked from commit 5a662f4)
The MoE gate and up projections consume the same normed activation, so the
forward-WHT + q8_1 rotation ran once per projection. Cache it per graph
eval (same key as the mmvq shared-quantize cache: tensor identity, data
pointer, size, epoch; main stream only) and return the cached buffer.

Removes ~40 kernel dispatches per token on a 40-layer MoE. At the measured
~4.4us cost of a small dispatch on MI210 (1.3us hardware floor plus
unhidable cold-miss latency for a kernel too small to fill the GPU) that is
worth about 0.18ms/token. GGML_TQ_ROTCACHE=0 disables.

(cherry picked from commit 7ef6f21)
…thmetic

tq4_cents8_reg expanded 8 nibble indices to 8 int8 centroids with ~100 ALU
ops of shifts and selects, about 400 per 32-weight block. That made the TQ
decode kernels ALU-bound rather than bandwidth-bound: the MoE matvec was
sustaining only ~24% of MI210 peak bandwidth while the dense Q8_0 matvecs
on the same card reached ~72%.

Use get_int_from_table_16 (__builtin_amdgcn_perm, the IQ4_NL/MXFP4 path) to
do the lookup in hardware: about 14 ops instead of 100.

The shift fallback existed because HIP __byte_perm() with a runtime selector
lowers through a dynamically indexed byte union that PromoteAlloca turns
into a 32KB LDS staging area. __builtin_amdgcn_perm has no such problem and
maps 1:1 to the instruction - this is the same idiom already validated in
the TQ4_1S MMQ tile loader.

(cherry picked from commit 080626d)
… and MUSA

tq4_cents8_reg() is on the dp4a decode path that NVIDIA also compiles, so the
unguarded AMD builtin broke the CUDA and MUSA builds. Keep v_perm_b32 on HIP
and use the equivalent __byte_perm selectors (0x5140 / 0x7362) elsewhere.

(cherry picked from commit 949de16)
One output row was spread over all 32 lanes, so each lane covered only
blocks_per_row/32 blocks and then paid a full 5-round warp reduction. The
reduction (5 dependent ds_bpermute, ~200 cycles) cost more than the work
(~150 cycles), which is why every attempt to add parallelism made things
worse: split-K was monotonically bad (115 -> 92 t/s at KS=8), 64-lane rows
lost 7%, and extra per-lane ILP was a wash.

Give each row 16 lanes instead: 4 reduction rounds, twice the work per
lane, and the warp still reads only two contiguous weight regions (going
below 16 scatters reads across more rows and loses more to coalescing than
it saves - 8 and 4 both measured worse). Never assign more lanes than there
are blocks, so no lane idles on narrow projections; previously the down
projection (16 blocks) left half the warp idle.

Paired A/B, alternating, 3 pairs: +3.1%, +2.6%, +3.0%. Output is
byte-identical to the old mapping under greedy decode.
GGML_TQ_LPR=N overrides.

(cherry picked from commit 625028a)
nvcc treats the unreferenced local as an error under -Werror all-warnings; the
cached pre-rotation helper computes the block count itself now.
@TheTom

TheTom commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Pushed a one-liner: nvcc rejected the now-unused n_blocks_total in ggml_cuda_mul_mat_id_tq (error #177-D under -Werror). Rest of the diff reads fine to me; I'll run the CUDA MUL_MAT / MUL_MAT_ID tq4_1s cases on a GB10 while CI runs, since the new __byte_perm interleave in tq4_cents8_reg is NVIDIA-only code that CI only compiles.

@TheTom

TheTom commented Sep 3, 2026

Copy link
Copy Markdown
Owner

GB10 (sm_121, driver 580.173.02), head fa870a1, nvcc build with -DCMAKE_CUDA_ARCHITECTURES=121:

test-backend-ops test -b CUDA0 -o MUL_MAT      1697/1697 (149 tq4_1s cases)
test-backend-ops test -b CUDA0 -o MUL_MAT_ID    979/979 (44 tq4_1s cases)
GGML_TQ_NATIVE=1 ... -o MUL_MAT_ID              979/979

So the __byte_perm interleave and the templated lanes-per-row kernel are correct on NVIDIA, not just compiling. Merging once the hosted CI jobs finish.

The small-batch TQ MUL_MAT_ID path is CUDA-graph capturable and a context can
hold several captured graphs, so freeing the cache buffer when it is outgrown
could leave a sibling graph replaying against pool memory that has been handed
out again. Push outgrown buffers (including the cross-device case, which used
to leak) onto a retire list drained at context teardown, the same shape as the
q8 quantize cache. Also key the allocation off ctx.device throughout, clamp
GGML_TQ_LPR to the instantiated lane counts, floor the computed default at 2,
and update the AGENTS.md pitfalls for the byte-permute LUT and graph capture.
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Sep 3, 2026
@TheTom

TheTom commented Sep 3, 2026

Copy link
Copy Markdown
Owner

Pushed 3df06b1 with the last review items so this can land without another round trip:

  • tq_rot_cache no longer frees an outgrown buffer. The small-batch TQ MUL_MAT_ID path is graph-capturable and a context can hold several captured graphs, so a sibling graph could replay against pool memory that had been handed out again. Outgrown buffers (including the cross-device case, which used to leak) go on a retired list drained at context teardown, same shape as your q8_cache in cuda: shared-quantize cache, residual and elementwise-chain fusions (generic half of #338) #343.
  • Allocation, record and retire all key off ctx.device (the helper mixed ggml_cuda_get_device() and ctx.device).
  • GGML_TQ_LPR only accepts 2/4/8/16/32 and warns once otherwise; any other value used to size the grid for one lane count and launch the 32-lane kernel. The computed default is floored at 2 for the same reason (ncols_x == 32 used to reach 1).
  • AGENTS.md: rewrote the __byte_perm pitfall (the GB10 and MI210 runs are the evidence now) and corrected the stale line claiming CUDA graphs are disabled for TQ MUL_MAT_ID.

Re-running the GB10 sweeps on this head; merging when those and CI are green.

@TheTom
TheTom merged commit c26baf1 into TheTom:feature/turboquant-kv-cache Sep 3, 2026
8 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA documentation Improvements or additions to documentation ggml

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants