perf(hip): compile DFlash GPU draft top-K kernel for HIP#488
perf(hip): compile DFlash GPU draft top-K kernel for HIP#488cheese-cakee wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
1 issue found across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
1a0f30a to
40d84e2
Compare
|
Rebased onto current On-HW validation (Radeon 8060S, gfx1151, wave32, ROCm 6.4.4): built All |
howard0su
left a comment
There was a problem hiding this comment.
suggest to cleanup. there are lots of CU files are shared between HIP and CUDA. We should not create hip.cu unless no other approach. This new pattern can confuse people.
The GPU top-K + log-prob extraction (geometric_draft_topk_cuda.cu) was compiled only on CUDA, so DFLASH_GPU_DRAFT_TOPK was a no-op on ROCm and AMD paid a per-step vocab x n_tokens D2H + CPU heap extract every speculation step. The kernel body is plain arithmetic (no tensor cores, no CUDA-only intrinsics), so the same .cu compiles unchanged for HIP. Compile geometric_draft_topk_cuda.cu directly with LANGUAGE HIP through the hip_compat <cuda_runtime.h> shim (the shared-.cu pattern already used by deepseek4_hc_cuda.cu), instead of adding a separate .hip.cu wrapper translation unit. Rename the backend guard macro DFLASH27B_HAVE_DRAFT_TOPK_CUDA -> DFLASH27B_HAVE_DRAFT_TOPK so the consumers (qwen35_dflash_target, test_dflash) are backend-neutral. test_draft_topk_cuda is now built on the HIP backend as well (CUDA spellings mapped by the same shim) and validated against the CPU reference on gfx1151 (Radeon 8060S, wave32, ROCm 6.4.4). Also qualify the README GPU-flag defaults to the server harness.
40d84e2 to
25a123d
Compare
| target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK_CUDA=1) | ||
| # and take the GPU draft top-K path instead of the CPU fallback. Same macro | ||
| # name as the HIP branch above (backend-neutral). | ||
| target_compile_definitions(dflash_common PUBLIC DFLASH27B_HAVE_DRAFT_TOPK=1) |
There was a problem hiding this comment.
shall we have a separate PR to remove this knob as it was default already? DFLASH27B_HAVE_DRAFT_TOPK
There was a problem hiding this comment.
in my opinion better as a separate PR so this one stays scoped to the HIP port
Summary
The DFlash GPU draft top-K + log-prob kernel (
geometric_draft_topk_cuda.cu, added in #434) was compiled only on the CUDA backend. On HIP/ROCm buildsDFLASH27B_HAVE_DRAFT_TOPK_CUDAwas never defined, soQwen35DFlashTarget::project_hidden_to_topkalways took the CPU fallback: a fullvocab x n_tokensdevice-to-host logits copy plus a CPU heap extract on every draft step. On a bandwidth-bound card that per-step D2H is a plausible contributor to the gfx1100 "~2x slower than llama.cpp" report in #457.This compiles the exact same kernel for HIP. The body is pure arithmetic (no tensor cores, no CUDA-only intrinsics), so it builds unchanged through the existing
hip_compatshim, the same wayflashprefill_kernels.hip.cuis built.Changes
server/src/common/geometric_draft_topk.hip.cu(new): a thin TU that#includes the shared kernel body and is compiled withLANGUAGE HIP+-I hip_compat. On CUDA builds the.cuis still added directly; on HIP only this wrapper is.server/hip_compat/cuda_runtime.h: add the threecuda*->hip*mappings the kernel needs and the shim did not yet cover (cudaPointerAttributes,cudaPointerGetAttributes,cudaMemoryTypeDevice).DFLASH27B_HAVE_DRAFT_TOPK_CUDA-> backend-neutralDFLASH27B_HAVE_DRAFT_TOPK, defined on both backends; update the two call sites (qwen35_dflash_target.cpp,test_dflash.cpp).server/CMakeLists.txt: registertest_draft_topk_cudaon the HIP backend too (GPU-vs-CPU parity test), mirroring the existing HIPtest_flashprefill_kernelswiring.README.md: correct the note that both flags are CUDA-only.How it works
DFLASH_GPU_DRAFT_TOPKnow takes effect on HIP: the kernel runs top-K + online-logsumexp directly on the logits device buffer and copies back only then_positions x Kresults, eliminating the per-step full-vocab D2H and CPU heap extract. Compiling one shared body for both backends keeps the GPU result bit-for-bit identical to the CUDA build and to the CPU reference it is validated against; the automatic CPU fallback on any device failure (out-of-range index, allocation failure, etc.) is unchanged, so the change is output-neutral by construction.Note on the sibling flag:
DFLASH_GPU_VERIFY_ARGMAXalready reads an in-graphggml_argmaxnode inverify_treeand was never gated on the CUDA-only macro, so it needs no port and already runs on HIP. The README is corrected accordingly.Performance
Not yet measured. The change removes a
vocab x n_tokens(Qwen3.5 vocab ~152k) D2H plus a CPU heap top-K per draft step; the win scales with how bandwidth-bound the card is. A/B recipe on an AMD card: baselineDFLASH_GPU_DRAFT_TOPK=0, treatmentDFLASH_GPU_DRAFT_TOPK=1,python server/scripts/bench_llm.py --bench HumanEval(temp 0), comparing decode tok/s and acceptance length.Limitations
hipPointerAttribute_t.typemember is spelledtypeon ROCm >= 5.0. All AMD targets in scope (ROCm 6.4.x on the gfx1151 CI runner, 7.x per the discrete-RDNA3 guidance in gfx1100 (RX 7900 XTX, ROCm 6.4.1): dflash_server ~2x slower than llama.cpp, DDTree speculation never engages, SIGSEGV at long context #457) usetype; older ROCm that only exposes.memoryTypeis not supported.Verification
DFLASH27B_HAVE_DRAFT_TOPK_CUDAreferences remain; guard macro resolves on both backends; new HIP TU and test registration mirror the shippedflashprefillHIP pattern.test_draft_topk_cuda, andDFLASH_GPU_VERIFY_ARGMAX=2dual-path mismatch mode) needs gfx1100/gfx1151. Requesting thegpu-tests-amdrunner build (it compiles the tree) or a maintainer proxy to confirm the HIP compile and parity before merge.