Skip to content

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

Open
raufaser wants to merge 1 commit into
Anbeeld:v0.4.7from
raufaser:pr-a-hip-only
Open

hip: ROCm/HIP-only perf (TOP_K wave32 hybrid, k-quant VDR)#153
raufaser wants to merge 1 commit into
Anbeeld:v0.4.7from
raufaser:pr-a-hip-only

Conversation

@raufaser

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, 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).

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 517/517 pass, same-seed decode byte-identical.

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 1568/1568 pass, KLD bit-identical vs baseline, 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).
  • ggml-cpu/llamafile/sgemm.cpp: llamafile gate n<2n<=8 (CPU file, no GPU impact).
  • tests/test-backend-ops.cpp: extra Q6_K/Q4_K/Q5_K + FA-head tuning shapes (test-only).

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

  • HIP: KLD trio bit-identical, same-seed decode byte-identical, KVarN ctest subset green.
  • CUDA (RTX 4090): baseline vs +PR builds produce identical test-backend-ops results (MUL_MAT 0 fails, TOP_K 0 fails in both) — PR-A provably does not change CUDA behavior.

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.

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