CUDA/HIP: chunked MFMA prefill kernel for GATED_DELTA_NET (CDNA) - #24561
CUDA/HIP: chunked MFMA prefill kernel for GATED_DELTA_NET (CDNA)#24561jadenmach2 wants to merge 1 commit into
Conversation
|
Thanks for the cc. Pulled it onto a 5090 : tests look good — GATED_DELTA_NET 44/44 passes. I can’t test MFMA directly, but noticed two small issues:
I’ll keep focusing on the CUDA side for now. Whether the NV and AMD paths should stay unified or split probably needs confirmation from the collaborators. Nice work. |
|
@ggerganov any updates on this ? |
ORippler
left a comment
There was a problem hiding this comment.
What prevents us from reusing existing shared logic in ggml_cuda_mma? This would
- Reduce maintainer burden, while
- Enabling this for both AMD and NVGPUs simultaneously
|
@ORippler My reasoning for keeping this CDNA-gated for now: the chunked rewrite is a win on MFMA but roughly neutral-to-regressive on NVIDIA in #22587's results, and the tuning (Wave64, LDS budget, occupancy, f16 MFMA path) is fairly CDNA-specific — so a unified kernel would likely compromise the AMD case. It's also gated strictly to CDNA, so the NVIDIA path stays untouched and can't regress. So I'd lean toward landing the CDNA kernel now as a self-contained win, and treating the shared ggml_cuda_mma / NVIDIA path as its own effort. That said, if you'd rather unify it through ggml_cuda_mma, I'm happy to do that — just wanted to put @ORippler Please lmk whatever you prefer |
|
ggml-side CUDA/HIP maintainer here. I am stretched quite thin and do not have the capacity to maintain a CDNA-only kernel. I am open to merging a kernel like this but only if the PR author takes over the long-term maintenance; if they are not responsive I will remove the kernel again. Long-term the only feasible solution for me would be an implementation using the interface in |
|
@JohannesGaessler Happy to take that on — I'll own long-term maintenance of this kernel and I won't go silent; ping me on regressions or issues and I'll be responsive. I'll also keep monitoring the PR and the op going forward. My reasoning for keeping it as a dedicated CDNA kernel is purely the AMD performance: the hand-tuned MFMA path lands a meaningfully larger speedup than I'd expect going through mma.cuh That said, I hear your long-term mma.cuh vision. So the next step is really your call: if you're willing to merge this as the CDNA kernel, I'll maintain it; if not, I'll do the mma.cuh refactor (unified NV+AMD) and push that instead. Just let me know which way you'd like to go and I'll proceed. |
I mean, if you give me a choice I'd much rather have a generic kernel using the interface in |
am17an
left a comment
There was a problem hiding this comment.
This will probably break MTP because of the partial rollback.
@JohannesGaessler Makes sense, I'll do the mma.cuh refactor for a generic NV+AMD kernel. Will push a revision |
|
@am17an |
|
Then IMO it does not make sense to add this kernel. It needs to conform to existing use cases, which is that it should be added for CUDA and it should work with MTP. |
|
@am17an |
cb9e5e8 to
7146c75
Compare
|
Ported to ggml_cuda_mma (CUDA + AMD, one kernel) — @ORippler @JohannesGaessler -- test-backend-ops -o GATED_DELTA_NET → 48/48 pass on MI250X (gfx90a, ROCm 7), including the chunked + Perf (MI250X, isolated kernel, chunked vs per-token, H=48 d=128): 512 tok 1.56×, 1024 1.59×, 2048 1.80×, 4096 @JohannesGaessler @ORippler I don't mind maintaining the kernel. Could you please review this PR? Happy to iterate on the dispatch threshold / any interface concerns. |
7146c75 to
0307fd8
Compare
|
@JohannesGaessler @ORippler Did you have a chance to review this ? |
JohannesGaessler
left a comment
There was a problem hiding this comment.
The code is as far as I can tell the output of a language model with minimal to no human oversight. I would only accept this in terms of code quality if someone with a proven track record volunteers to take over the maintenance. However, given that the posts by the OP are clearly machine generated as well I do not have the necessary confidence in them for this role. So as it is I think it is not in our interest to merge this PR. FWIW I can confirm that I saw a 5-10% E2E improvement from this kernel when I tested it on my NVIDIA/AMD hardware.
|
Thanks for taking the time to test it and for the feedback, I appreciate you confirming the 5-10% E2E gain on your hardware. Yes, I used AI as a tool while writing this, but not without oversight, I reviewed, tested, and iterated on the kernel myself. The heavy commenting isn't auto-generated filler; I added it for my own sake because I had to revisit the chunked math and the MMA mapping repeatedly during the refactor, and it helped me keep the state/rollback logic straight. I'm happy to strip it down to the level that's normal for this codebase. I understand the maintenance concern and I don't want to hand you a PR you'd have to babysit. If you think the kernel is worth having, I'm glad to clean it up (comments, style, whatever review points you raise) and stay responsive on it. If you'd still rather not take it on given that, I completely understand and am fine closing the PR. Just let me know your final call so I know whether to start the cleanup or close it out. Either way, thanks for the review. |
|
I will for now close this PR then unless someone volunteers for maintenance. |
CUDA/HIP: chunked MFMA prefill kernel for GATED_DELTA_NET (CDNA)
Implements the chunked prefill kernel tracked by the
//TODO: Add chunked kernel for even faster pre-fillinggml/src/ggml-cuda/gated_delta_net.cuand discussed in #22967,scoped to AMD CDNA / MFMA (gfx90a/MI250X) and the non-KDA scalar-gate path.
cc @leonardHONG — saw you wanted to take the chunked CUDA prefill kernel in
#22967. I had a working CDNA/MFMA prototype with benchmarks, so I'm opening this as a
draft to share the implementation and numbers rather than duplicate effort. Happy to
fold it into your structure, or keep it as the AMD-specialised path alongside an NVIDIA
tensor-core version. Not trying to step on the issue — just contributing data + code.
What this does
Replaces the token-sequential recurrence with the standard chunkwise gated delta rule
(per Yang et al. "Parallelizing Linear Transformers with the Delta Rule…" and the
flash-linear-attentionreference), chunk size C=16. Within a chunk the recurrence isexpressed relative to the chunk-start state
S_{t0}so the heavy work becomes densematmuls:
KKᵀ,QKᵀ(C×C)K@S,Q@Sprojections (C×S_v · S_v×S_v)(I+L)Δ=Ulower-triangular solve (C steps, parallel across the 128 state columns)Kᵀ@Wstate updateThe three GEMMs use
__builtin_amdgcn_mfma_f32_16x16x16f16(f16 in, f32 accumulate;K=16/issue, 4× the f32_16x16x4 path). Numerical stability comes from keeping the gate in
log space and only exponentiating the bounded difference
Glog[t]-Glog[s](≤ C·max|log g|),which also keeps the f16 GEMMs within the op tolerance.
Scope / safety
Dispatched only when all of: non-KDA,
S_v==128,amd_mfma_available(cc)(CDNA),Wave64,
n_tokens ≥ 512, and enough grid blocks to hide the per-block setup. Everythingelse — NVIDIA,
S_v≠128, short sequences, few heads, KDA, keep_rs — falls through to theexisting per-token kernel, so no other path can regress.
#ifdef AMD_MFMA_AVAILABLE(HIP+CDNA only); each blockhas a scalar fallback, so the kernel still compiles on NVIDIA/CPU builds.
amd_mfma_available()returns false off-CDNA, so chunked is never selected there.n_tokens=1) never takes the chunked path → TG unaffected by construction.GGML_CUDA_DISABLE_DELTANET_CHUNKED=1forces the per-token kernel.GGML_CUDA_DELTANET_{CHUNKED_MIN_TOKENS, MIN_BLOCKS_PER_SM,CHUNK_C,NCOLS}.Correctness
test-backend-opsGATED_DELTA_NET cases pass, including added partial-chunk (17, 65),multi-seq, and the H=48/d=128 Qwen3.x prefill geometry. Max abs diff vs the per-token
kernel ~1.2e-4 (fp32 round-off).
Benchmarks (MI250X / gfx90a, single GCD unless noted)
Isolated kernel (
test-backend-ops perf, H=48 d=128 — the low-noise ground truth),chunked vs per-token:
Chunked sustains ~46 GB/s; per-token degrades 30→18 GB/s. The gap is larger than the
NVIDIA chunked-vs-AR numbers in #22587, reflecting how much MFMA the AR path leaves idle
on CDNA.
End-to-end (Qwen3.6-27B Q4_K_M), chunked vs per-token:
Under the realistic default ubatch (each kernel call sees ≤512 tokens) the E2E gain is
the more modest +2–6%; the large gains need a big ubatch so the kernel sees the full
prompt. This is consistent with the "delta-net is ~2–3% of E2E, hard to measure" point in
#22587 — hence the isolated-kernel table above as the primary evidence.
4-GCD layer-split run confirms the per-GCD A/B delta survives sharding (+6 → +21%);
default-ubatch throughput scales ~3× via microbatch pipelining with the chunked win intact.
Known limitations / open questions
S_v=128+ non-KDA only. KDA and otherS_vstay on the per-token kernel.engineering + dispatch gating, not the formulation.
and on whether to align this with an NVIDIA tensor-core implementation.