Make KVarN usable on ROCm RDNA3 (toward #122: D256 race, portable prefill, f32 WMMA prototype) - #149
Make KVarN usable on ROCm RDNA3 (toward #122: D256 race, portable prefill, f32 WMMA prototype)#149raufaser wants to merge 17 commits into
Conversation
flash_attn_ext_f16_process_tile reuses tile_Q as combine staging across k00 iterations; the end-of-iteration barrier only fired for np > 1. D256/ncols=64 (nbatch_combine=64, DV/2=128, np=1) corrupted output. Sync on all but the last iteration, plus sync between process_tile calls that reuse tile_Q. Enable D256 in the RDNA WMMA route policy and device guard.
RDNA WMMA VKQ accumulators are fp16 (~3e-4/call vs ~1e-5 portable), compounding through depth into a KLD collapse. Prefer portable-native direct-record attention for HIP prompt-prefill (nq > 16); decode stays on WMMA. Opt out with GGML_KVARN_AMD_PROMPT_PORTABLE=0.
Replace the per-token 7-stage shared-memory reduction tree with warp shuffles plus one cross-warp step; rescale the half accumulator in fp32; unroll the V-load loop.
GGML_KVARN_TEST_NKV_LADDER_ONLY runs GPU-native vs CPU-materialized attention error over D128/256/512 and growing n_kv.
Serve QB=8 queries per block from one shared K/V token stream (no-tail path; tail keeps QB=1): ~2.7x prefill (pp4096 107 -> 291 t/s) at ladder-identical accuracy. Also replace the per-token shared-memory reduction tree with warp shuffles, resolve each token once per block instead of per thread, and add an env-gated kernel attribute print (GGML_KVARN_PORTABLE_ATTRS).
RDNA3 WMMA VKQ accumulators are fp16 (~3e-4/call), compounding over depth into a KLD collapse (2.14 at 32k). Mirror the proven DV=80/112 fp32-PV tiles for DV=128/256: 16-wide A, one wmma_f32 per K step, persistent f32 accumulator. Also fix the RDNA3 VKQ_C entry count for 16-row A tiles (was sized for 32-row tiles); the count is unchanged for all pre-existing configs. gfx1100, Qwen3.6-27B-Q5_K_S, kvarn6: ladder RMSE ~1e-5, 32k KLD 0.022 vs portable 0.023 (was 2.14), same-top 97.0 percent, prefill pp4096 286 -> 497 t/s. Gated behind GGML_KVARN_AMD_PROMPT_PORTABLE=0; default serving path unchanged.
|
Please change the target branch to v0.4.7, and ensure changes are compatible with it. |
The three direct final-meta stores in flash_attn_ext_f16_process_tile were gated '!is_kvarn_kv', so KVarN whole-tile blocks never wrote body_meta (softmax max/rowsum). Split tiles get meta from the stream-k fixup, but flash_attn_stream_k_fixup_general skips tiles whose K range aligns exactly to tile boundaries; those rows kept zero meta (den=0) and the tail merge silently discarded their correct body values. Symptom: KVarN WMMA prompt path clean at ub<=64, garbage at ub>=96. Removing the gate makes whole-tile blocks publish their (max, rowsum) like dense FA. Validated: ub512 KLD on 4B/27B/35B-MoE all at portable parity; 32k KLD 2.118 -> 0.029.
# Conflicts: # ggml/src/ggml-cuda/fattn-kvarn-portable.cuh
…s standard) Make F32-WMMA direct-record attention the default for HIP KVarN prompt-prefill (was: portable-native by default, WMMA opt-in). The ub-geometry fix (whole-tile body_meta) plus the DV=128/256 fp32 accumulators make WMMA both the fast and the exact route (~1e-5 ladder RMSE, 32k KLD at portable parity), validated at the serving default (-ub 512): 4B KLD 0.006822 (bit-exact vs fp16 base), 27B 0.004814, 35B-MoE 0.003526. Portable-native remains for unsupported shapes and as an explicit opt-in (GGML_KVARN_AMD_PROMPT_PORTABLE=1). Decode (nq<=16) unchanged. HIP-gated; CUDA/Vulkan and all non-KVarN paths untouched.
The GGML_KVARN_PORTABLE_ATTRS debug print used hipFuncAttributes / hipFuncGetAttributes unconditionally, which do not exist on CUDA and broke the CUDA compile (even though the block only runs when the env var is set). Use the HIP API under GGML_USE_HIP and the CUDA API (cudaFuncAttributes / cudaFuncGetAttributes, same fields) otherwise. No behavior change on HIP; CUDA now compiles. Found while validating the F32-WMMA flip on thermis/4090.
Resolve portable.cuh by taking upstream's complete optimized D64 rewrite. Our interim D64 hand-merge (RECORD_DIM/v_original_domain port) and portable QB-batching/warp-shuffle are superseded (upstream now covers D64 natively; portable is fallback-only post-flip so peak fallback perf is acceptable; re-propose batching as a follow-up if fallback perf matters). Our CUDA func-attributes fix is not needed (upstream removed the HIP-only debug block). Preserved: F32-WMMA default flip (dispatch, auto-merged) and ub-geometry fix (fattn-mma-f16, auto-merged).
Follow-up to the v0.4.7 (complete optimized D64) merge, which superseded our interim portable QB-batching: the flip's fallback dispatch still called the now-removed ggml_cuda_flash_attn_ext_kvarn_portable_batched (undeclared on the merged tree). Route the fallback through the standard portable kernel (correct, unbatched). Portable is fallback-only post-flip.
|
Requesting changes, one blocking item on RDNA4 plus four smaller asks. Reviewed at 1. RDNA4 gets the half2 tiles the fp32 work was meant to replace (blocking)
Please gate the raised limit on the arches that actually compile the fp32 tiles (the D64 path already does this per-backend fail-closed clearing at 2. The D128 fp32 tile also changes dense HIP attention
3. The regression guard for this geometry does not run on HIP
4. The portable opt-in still pays for the WMMA pass
5. Document the new knob
What I verified and did not: this was a read-only review; no build or test ran, and there is no ROCm toolchain on this machine, so the AMD paths are neither compiled nor executed here. Your gfx1100 ladder, KLD and bench numbers are taken as reported. The merge with the current base tip is clean, but Checked and found sound, in case it saves time: the |
…nto kvarn-rocm-v0.4.7
|
Thanks for the careful read — all five addressed in 1. RDNA4 fail-closed (blocking): fixed as suggested (gate, not tile port). 2. Dense D128: evidence (no scoping). 3. HIP tail coverage: provided. 4. Opt-in double pass: fixed. 5. Knob docs: done. Re-validation on the merged tree (tip |
|
Thanks for the turnaround on the five items. I checked the new head 1. Drop the config rows no route can reach, or name the route that reaches them. 2. Give the one live row a number. 3. Fix the 4. Make the HIP tail coverage a committed test, not a manual run. 5. Pin the portable original-V-domain path. I verified the 6. Close or scope the dense D128 gap. The Update the feature matrix too. Out of scope, please file separately: the D512 + attached-tail After these I have no remaining blockers on the change itself. For the record: my review is read-only. There is no ROCm toolchain on this machine, so the HIP and AMD paths are neither compiled nor run here, and all of your gfx1100 and RTX 4090 numbers are taken as reported. |
Retune the RDNA (256,256,64) WMMA tile config to the validated values (measured KLD-clean on gfx1100 prompt-generic-mma) and raise the AMD WMMA device guard accordingly, so KVarN D256 shapes route to the fast WMMA path instead of the portable fallback. Only the (256,256,64) row is reachable (the KVarN switch has 128/256/512, standard FA caps RDNA at 128, eligibility caps RDNA3 at 256); the 320/512/576 keys in this commit are unreachable and reverted by the follow-up. Also size the portable kernel shared scratch by head dim (reduction[D] / transform[D]): the D64/D256/D512 V-domain transform indexes full-head dims into arrays previously sized RECORD_DIM, an out-of-bounds access for D256/D512. Latent on HIP (always rotated domain) but reachable on CUDA portable with the original V domain. Validation (gfx1100, kvarn6, ub512 KLD vs fp16): 27B 0.0046/95.1% via prompt-generic-mma with zero kernel traps (was 10.8 garbage); 35B-MoE 0.0037/97.1%; 4B 0.0072/96.9% unchanged. CUDA 4090: 27B 0.0033, KVarN/FA ctest 16/16, backend-ops MUL_MAT + TOP_K clean.
…docs 1. RDNA4 stays fail-closed at D128: the fp32-accumulator tiles that justify the raised D256 limit compile under RDNA3 (gfx11) only, so a new RDNA4_WMMA arch keeps the 128 head-dim cap (D64-style fail-closed) while RDNA3/3.5 admit 256. Dispatch comment corrected to RDNA3 scope. Route-policy test covers RDNA4-256 rejection. 2. Portable opt-in checked before the generic probe, so GGML_KVARN_AMD_PROMPT_PORTABLE=1 no longer pays for a discarded WMMA pass. Boolean aligned to nonzero-means-set (was strict ==1). 3. Knob documented in docs/beellama-args.md with exact accepted values. 4. D256 WMMA prefill with attached tail is covered on HIP via the ladder (GGML_KVARN_TEST_LADDER_TAIL=128: D256 n_kv 256-8192 RMSE ~1e-5 on the WMMA route); D512+tail hits a pre-existing fail-closed identical on the pre-change build.
…tted tests - Drop the unreachable 320/512/576 retunes (no route selects those keys: KVarN switch has 128/256/512, standard FA caps RDNA at 128, eligibility caps RDNA3 at 256) and restore the device guard to DKQ > 256, mirrored by host eligibility again. Only the measured (256,256,64) row stays. - Scope the RDNA3 D128/D256 fp32 PV-accumulator tile to the KVarN path (mma_tile_sizes kvarn_accum gate): dense HIP attention keeps the qualified half2 tile, closing the dense-D128 evidence gap structurally. - Committed regression tests: D256 WMMA prompt prefill with attached exact tail vs CPU reference (ub-fix coverage), and portable with original_value_domain=true at D256/D512 vs CPU (shared-scratch fix coverage, force-pinned to the portable route). - AMD route-boundary expectation updated for admitted D256; feature matrix splits RDNA3/3.5 (WMMA to D256) from RDNA4 (D128 + portable).
…mmitted tests - Enable portable for ROTATED_K_ORIGINAL_V at every head dim (was D64-only): the full-head V inverse transform inverts the stored forward transform, validated vs CPU at D256/D512 RMSE ~5e-4. Previously the portable+MIXED combination fell through to abort. - Scope the RDNA3 D128/D256 fp32 PV-accumulator tile to the KVarN path via mma_tile_sizes_kvarn (dense keeps its qualified half2 tiles). - Committed regression tests: D256 WMMA prompt prefill with attached exact tail (route-pinned, meta-denominator assertions, serving-geometry parity) and portable with original_value_domain=true at D256/D512 vs CPU (route- pinned via force-portable env). AMD route-boundary expectation updated for admitted D256.
bebc08f to
f7f3350
Compare
|
All six items are closed on f7f3350 (force-pushed; 4 commits on top of bebc08f). Per-item, with the acceptance evidence:
Feature matrix: RDNA3/3.5 vs RDNA4 rows are split and read the same in both files. CUDA gate on the final tree (RTX 4090, clean rebuild): 16/16 ctest incl. test-kvarn; 27B kvarn6 0.0042-0.0044/97.5-97.8% on the default route (round-1 default was 0.0062/96.3% against its own base); backend-ops MUL_MAT+TOP_K 2190 cases OK. D512+tail filed as #158. Note for the record: the fattn-tail.cuh:713 abort reproduces on base and mid-branch trees but no longer reproduces on f7f3350 (tail ladders 128 and 256 fully green) - details in the issue. |
Overview
Toward #122 (KVarN on ROCm is unusable: wrong results plus prefill far behind). This branch makes KVarN work on RDNA3 step by step, validated on gfx1100 (RX 7900 XTX, ROCm 7.14) with Qwen3.6-27B-Q5_K_S and kvarn6:
D256 k00-combine race fix in
fattn-mma-f16.cuh(tile_Q reused across k00 iterations, barrier only fired for np > 1), plus inter-tile sync. Enables the RDNA D256 route.HIP prompt-prefill (nq > 16) routes through portable-native direct-record attention, since RDNA WMMA VKQ accumulates in fp16 (~3e-4/call) and compounds over depth into a KLD collapse. Decode stays on WMMA. Opt out with
GGML_KVARN_AMD_PROMPT_PORTABLE=0.Portable prefill optimizations (warp-shuffle reduction, fp32 rescale, QB=8 queries per block, hoisted token resolve): pp4096 107 to 291 t/s at identical accuracy.
Accuracy ladder harness (
GGML_KVARN_TEST_NKV_LADDER_ONLY): GPU-native vs CPU-materialized error over D128/256/512 and n_kv 256 to 8192.f32 WMMA accumulator prototype for DV=128/256 prefill (16-wide A, single
wmma_f32per K step, persistent f32 acc, mirroring DV=80/112), plus theVKQ_Centry-count fix it required. Still gated behindPORTABLE=0, default serving path unchanged.ub>64 fix: the three whole-tile
dst_final_metastores inflash_attn_ext_f16_process_tilewere gated!is_kvarn_kv, so KVarN whole-tile blocks never published their (max, rowsum). Split tiles got meta from the stream-k fixup, but that skips boundary-aligned tiles, so those rows kept zero meta and the tail merge silently discarded their correct body values. Symptom: KVarN WMMA prompt path clean at ub<=64, garbage at ub>=96. Removing the gate makes whole-tile blocks publish their meta like dense FA. Validated: ub512 KLD on 4B / 27B / 35B-MoE at portable parity (4B bit-exact); 32k KLD 2.118 -> 0.029.Results: ladder RMSE ~1e-5; 32k KLD 0.022 vs portable 0.023 (was 2.14 on the fp16 path), same-top 97.0%; prefill pp4096 286 (portable) vs 497 (f32-WMMA); decode ~33.5 both; reference q8_0 at 804.
Base: v0.4.7
Retargeted to
v0.4.7per maintainer request. The branch mergesv0.4.7(D=64 KV-head support, etc.) with this work; the only merge conflict wasfattn-kvarn-portable.cuh, resolved by porting v0.4.7's D=64 support (RECORD_DIM, thev_original_domainoriginal->rotated V Hadamard, D=64 dispatch) onto the portable QB-batching + warp-shuffle kernel.fattn-kvarn-dispatch.cu: D64 stripped from capabilities on non-CUDA backends "until each backend has independent runtime qualification"). So on this gfx1100 build D64 is never dispatched; the D=64 kernel here is compile-validated and mirrors v0.4.7's, but runtime qualification on HIP is out of scope. D128/256/512 are the validated ROCm paths.Additional information
Progress toward Bug Report: kvarn6 KV-quantization causes massive performance regression on ROCm (RX 7900 XTX) #122 (update posted there during the work).
The f32 approach follows https://github.com/stew675/llama-cpp-rdna-boosts (question thread; their answer pointed at the DV=80/112 fp32-PV tiles as reference).
Validate with:
ctestKVarN subset, ladder env above,llama-perplexityKLD flow andllama-bench -p 4096from AGENTS.md.Known, kept out of scope: the f32-WMMA default flip (still behind
PORTABLE=0); two flaky AVs under long KLD runs share one fault offset with dumps saved, zero occurrences on portable,PORTABLE=1is the fallback. (The ub>=96 serving divergence previously listed here is now fixed by the ub>64 commit above.)Requirements
I have read and agree with the contributing guidelines
AI usage disclosure: YES. An AI coding assistant (Muse Spark supervised by the author) wrote the tile-spec and count changes plus test-harness knobs, ran the builds, ladder/KLD/bench measurements on the author's RX 7900 XTX, and drafted this description. All numbers above are measured, not estimated; the author reviewed the diff before submitting.
Update 2026-09-09: F32-WMMA is now the default + CUDA build fix (2 new commits)
This supersedes the "f32-WMMA default flip (still behind
PORTABLE=0)" note above.bad6f86c1Ôö£├ÂÔö£├ºÔö£├é F32-WMMA is now the default for HIP KVarN prompt-prefill(was: portable-native by default, WMMA opt-in). The ub-geometry fix plus the
DV=128/256 fp32 accumulators make WMMA both fast and exact, validated at the
serving default (
-ub 512, gfx1100, kvarn6, ngl99): 4B KLD 0.006822 (bit-exactvs fp16 base), 27B 0.004814, 35B-MoE 0.004093; 32k KLD 0.029 (was 2.118);
ladder D128/256/512 ~1e-5. Portable-native remains for unsupported shapes and
as an explicit opt-in (
GGML_KVARN_AMD_PROMPT_PORTABLE=1). Decode (nq<=16)unchanged. HIP-gated; CUDA/Vulkan untouched.
edc1d2f64Ôö£├ÂÔö£├ºÔö£├é CUDA build fix (pre-existing, found by the thermis RTX 4090build): the
GGML_KVARN_PORTABLE_ATTRSdebug print used HIP-onlyfunc-attribute APIs, breaking nvcc. Now HIP API on HIP / CUDA API on CUDA
(debug-only, no behavior change). **This PR as-merged did not build on CUDA;
this fix is required.**
CUDA gate (thermis RTX 4090,
GGML_CUDA=ON): full build clean; KVarN/FA ctest(
kvarn|fattn-route|cuda-fattn) 14/14 pass, incl.test-kvarn. The flip iscompiled out on CUDA (HIP-gated) Ôö£├ÂÔö£├ºÔö£├é expected no-op, confirmed.
Community validation needed: gfx1151 (Strix Halo) / gfx1201 (RDNA4)
We only have gfx1100 (RX 7900 XTX) plus CUDA/4090. If you run **gfx1151 or
gfx1201**, please validate this PR on your hardware and report back:
Build clean (
GGML_HIP=ON,GPU_TARGETSset for your arch at initial configure).The ub512 KLD trio (see "Validate with" above) Ôö£├ÂÔö£├ºÔö£├é especially whether F32-WMMA
as-default is accurate at your serving
-ub(state the-ubvalue).The accuracy ladder (
GGML_KVARN_TEST_NKV_LADDER_ONLY=1).ctest -R "kvarn|fattn-route".RDNA4 note: D64 KVarN is CUDA-only by design (fail-closed on HIP); D128/256/512
are the expected ROCm paths. Thank you!
Update 2026-09-09 (pm): merged v0.4.7 "complete optimized D64", conflict resolved
origin/v0.4.7advanced (e1f6d6fe6, complete optimized D64) after this PR'searlier v0.4.7 merge, conflicting in
portable.cuh. Resolved by merging(
cc566c899) and taking upstream's complete D64 rewrite.Our interim D64 hand-merge + portable QB-batching/warp-shuffle are superseded
(upstream covers D64 natively; portable is fallback-only post-flip, so peak
fallback perf is acceptable; re-propose batching as a follow-up if it matters).
Our CUDA func-attributes fix proved unnecessary (upstream removed the HIP-only
debug block).
Preserved through the merge: the F32-WMMA default flip and the ub-geometry fix
(both auto-merged). One 3-line follow-up (
37ce9e337) drops the now-deadbatched-portable call (fallback uses standard portable ├ö├ç├ correct, unbatched).
Validation on the merged tree (gfx1100, default WMMA): ub512 trio identical to
pre-merge (4B 0.006822 bit-exact, 27B 0.004814, 35B 0.004093); key-target build
clean. PR is MERGEABLE/CLEAN.
Follow-up: validated RDNA D256 tiles + portable shared OOB fix (570cb2c)
Update 2026-09-11: review round 2 addressed (force-pushed as f7f3350)
Commits on top of the previous pushed head (bebc08f):
Validation on the final tree (clean rebuild from f7f3350, gfx1100 RX 7900 XTX, kvarn6, ngl99, -ub 512):
Two honest caveats: