Skip to content

hip: ROCm/HIP-only perf (TOP_K wave32 hybrid, k-quant VDR) - #153

Merged
Anbeeld merged 2 commits into
Anbeeld:v0.4.7from
raufaser:pr-a-hip-only
Sep 12, 2026
Merged

hip: ROCm/HIP-only perf (TOP_K wave32 hybrid, k-quant VDR)#153
Anbeeld merged 2 commits into
Anbeeld:v0.4.7from
raufaser:pr-a-hip-only

Conversation

@raufaser

@raufaser raufaser commented Sep 10, 2026

Copy link
Copy Markdown

ROCm/HIP-only performance work. Every hunk in this PR is #if defined(GGML_USE_HIP)- / RDNA-arch-gated, CPU-only, or test-only. The sgemm hunk is back to upstream after review, so CUDA/Vulkan/CPU behavior is byte-identical. Verified with a CUDA negative control: clean-v0.4.7 baseline vs baseline+this-PR -- test-backend-ops MUL_MAT and TOP_K all-pass in both states (identical outcomes: 1818/1818 each).

What's inside (all HIP/RDNA-gated)

1. TOP_K wave32-native + hybrid kernels (ggml-cuda/top-k.cu)
Idea from nasone32's RDNA3-7900xtx-opt ("Applied series"). Replaces the no-CUB HIP radix fallback with wave32-native tiled + hybrid kernels, keeping the CUB preference where CUB exists. Op-level: TOP_K ne=[200000,1,1,1] k=40 runs on GPU in ~86.7 us vs ~161.2 us on the CPU fallback path. Validation: test-backend-ops TOP_K 525/525 pass (ROCm0, gfx1100), same-seed decode byte-identical. Review fix: per-iteration seeding of the radix selection state, which stayed uninitialized (first pass) / stale (later passes) for all-+inf/NaN rows.

2. RDNA k-quant VDR decode boosts (ggml-cuda/mmvq.cu, ggml-cuda/vecdotq.cuh)
Idea from stew675's llama-cpp-rdna-boosts block 10 (RDNA-gated VDR bumps for Q4_K/Q5_K/Q6_K/Q8_0). Decode tg64 +4-6% on gfx1100. Validation: test-backend-ops MUL_MAT 1293/1293 pass (372 unsupported-type skips of 1665 selected cases), KLD identical vs the v0.4.7 base (see table), same-seed decode byte-identical. (The new *_vdr4/*_vdr2 entry points compile on all backends but are dead code outside the RDNA dispatch gate -- confirmed by the CUDA-build negative control.)

3. Small HIP-gated + neutral hunks

  • ggml-cuda.cu: integrated=false device flag under GGML_USE_HIP (the CUDA #else arm already was false -- no behavior change). Review fix: the "at no cost" comment now records the placement trade-off, with a matching note in docs/beellama-features.md (APU tensors move off host-mapped memory, VRAM headroom changes, unmeasured -- no APU hardware here).
  • ggml-cpu/llamafile/sgemm.cpp: REVERTED to upstream if (n < 2) per review -- the CPU neutrality claim above is exact again.
  • tests/test-backend-ops.cpp: extra Q6_K/Q4_K/Q5_K + FA-head tuning shapes (test-only); removed one duplicated dead Q8_0 line.

Validation (gfx1100 RX 7900 XTX + RTX 4090 CUDA gate)

  • HIP test-backend-ops: TOP_K 525/525, MUL_MAT 1293/1293 (ROCm0).
  • CUDA negative control (RTX 4090, sm_89): baseline vs +PR builds produce identical test-backend-ops results (1818/1818 in both, byte-identical outcomes) plus ctest -R "kvarn|fattn-route" 15/15 -- this PR provably does not change CUDA behavior.
  • KLD trio (kvarn6, -c 512 -ub 512, ngl99), stock v0.4.7 / this PR / PR hip: shared perf (MMQ MoE tiles + mmvdq, prefill plumbing) #154 -- bit-identical across all three trees:
model KLD same-top
4B Q4_K_M 2.197615 61.961%
27B Q5_K_S 0.004916 95.490%
35B-A3B 0.004116 97.843%

Kudos

  • nasone32 (llama.cpp-RDNA3-7900xtx-opt) for the wave32 TOP_K hybrid design this ports.
  • stew675 (llama-cpp-rdna-boosts, block 10) for the RDNA k-quant VDR tuning this ports.
  • Porting, validation, and CUDA negative control by raufaser; numbers measured on RX 7900 XTX (gfx1100) unless noted.

Base: v0.4.7. Sibling PR (shared-code perf) follows separately as PR B.

ROCm/HIP-only performance work with zero CUDA/Vulkan impact. Every hunk is
`#if defined(GGML_USE_HIP)`- / `#if defined(RDNA*)`-gated, CPU-only, or
test-only; CUDA paths are byte-identical by construction.

- top-k.cu: replace the v0.4.7 no-CUB HIP radix fallback with the wave32-native
  hybrid (n-ary search + small-kernel + parallel radix + bitonic), gated
  `!CUB && HIP` so CUB is still preferred when available. (Task 3; supersedes
  the simpler v0.4.7 radix.)
- mmvq.cu / vecdotq.cuh: RDNA Q4_K/Q5_K VDR=4, Q6_K VDR=2, Q8_0 VDR=4
  (RDNA-gated macros/dispatch; new VDR kernels defined but dead on CUDA).
  (Task 5)
- ggml-cuda.cu: HIP APU `integrated=false` host-buffer back-out (HIP-gated;
  CUDA branch unchanged). (Task 6/06)
- sgemm.cpp: CPU llamafile decode/verify parity (`n<=8` through vec_dot).
  (Task 6/05)
- tests: Qwen3.6-27B Q6_K/Q4_K/Q5_K + FA-head tuning shapes (test-only).

The Task 2 FA head-cap/tile cluster (fattn.cu selection + fattn-mma-f16.cuh
kernel cap + tile configs) is coupled and lives in PR-B, not here, to keep
this PR self-consistent and provably CUDA-safe.

CUDA negative control (thermis/4090) to be recorded in the handoff; gfx1100
task records: TOP_K 517/517 backend-ops + same-seed byte-identical, VDR
MUL_MAT green + KLD identical + tg64 +4-6%.
@Anbeeld

Anbeeld commented Sep 10, 2026

Copy link
Copy Markdown
Owner

Requesting changes on the CPU dispatch hunk; the rest are non-blocking.

Blocking: ggml/src/ggml-cpu/llamafile/sgemm.cpp:3826 (was if (n < 2) at 3819). Changing the guard to n <= 8 is not HIP-gated. It runs at the top of llamafile_sgemm before the type switch, GGML_LLAMAFILE is on by default, and __MMA__ is never defined on x86-64 or ARM, so every CPU build now sends n = 2..8 to the generic vec_dot path instead of tinyBLAS (ggml-cpu.c:1342,1417). That range is what a speculative verify batch uses (n_draft+1), and it is also what -b/-ub in 2..8 selects for prompt processing. The PR body says CPU behavior is byte-identical, which cannot hold for this hunk. Please either point at the fork requirement or test that needs batch-versus-sequential bit-identity (I could not find one; tools/server/README.md:530 documents that logits are not bit-identical across batch sizes), scope the change to that validation path, or move it to the shared PR with CPU measurements. If it stays, drop the neutrality claim and record the n = 2..8 CPU effect.

Non-blocking:

  • ggml/src/ggml-cuda/top-k.cu:245-256: selected_bucket and selected_total are written only when __ffsll(...) - 1 matches a lane. If both ballots are zero, which happens when the current range holds fewer than limit keys (an all-+inf/NaN logits row, since ordered keys at or above 0xff800000 are excluded), then bucket = selected_bucket reads uninitialized shared memory on the first pass or stale values on later passes. The outcome is wrong indices, not memory unsafety. Seeding the state per iteration would close it.
  • ggml/src/ggml-cuda/ggml-cuda.cu:315: integrated = false on HIP also changes placement, because ggml_backend_cuda_device_supports_buft (ggml-cuda.cu:5827) gates the CUDA-host buffer type on that field. On APUs this can move tensors off host-mapped memory and change VRAM headroom, so "at no decode/prefill cost" understates it. Worth recording in docs/beellama-features.md next to the measured PPL 5.9243 to 8.51+ corruption.
  • tests/test-backend-ops.cpp:10086-10087: the Q8_0 17408x512 line is added twice, inside the #if 0 region at 10075-10086, so it is dead code.

What I checked: the new Q4_K/Q5_K VDR=4 and Q6_K VDR=2 entry points match the canonical VDR=2/VDR=1 element partition, the q8_1 block pairing and scale/d8 indices, and the kqs = vdr * (tid % (qi/vdr)) stride. No builds or tests were run, so the perf and byte-identity numbers in the description are unverified here.

@raufaser
raufaser requested a review from Anbeeld as a code owner September 11, 2026 20:33
@raufaser

Copy link
Copy Markdown
Author

All items addressed in 5344385 (on top of abe3026, no force-push). Builds + validation below are new runs on the final tree: HIP = RX 7900 XTX gfx1100 (TheRock, clean GPU_TARGETS=gfx1100 configure), CUDA = RTX 4090 sm_89.

Blocking (sgemm): reverted to upstream if (n < 2) -- the batch-vs-seq comment block is gone, so the neutrality claim in the body is exact again. No CPU measurement needed (upstream path restored); the CUDA negative control below proves no impact anywhere.

  • top-k.cu: per-iteration seeding of selected_bucket/selected_total before the ballot section. Safe for normal rows (any nonzero ballot overwrites before the broadcast read). test-backend-ops -o TOP_K: 525/525 pass on ROCm0.
  • ggml-cuda.cu: the "at no cost" claim is struck -- the comment now records that supports_buft gates the CUDA-host buffer type on integrated, so APU tensors move off host-mapped memory and VRAM headroom changes (unmeasured, no APU hardware here). Matching note added to docs/beellama-features.md next to the HIP APU paragraph.
  • test-backend-ops.cpp: deleted one copy of the duplicated dead Q8_0 line.

Validation table:

The only build failure is the known pre-existing test-kv-history.exe link error on Windows (documented v0.4.7 quirk, also fails on the reference build); everything else links, including all validation binaries.

@Anbeeld
Anbeeld merged commit 09c3d39 into Anbeeld:v0.4.7 Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants