Skip to content

(WIP) CUDA/HIP: GPU partial top-k for ROCm (fix full-sort/CPU fallback) - #27860

Closed
zihaomu wants to merge 2 commits into
ggml-org:masterfrom
zihaomu:feat/hip-partial-topk
Closed

(WIP) CUDA/HIP: GPU partial top-k for ROCm (fix full-sort/CPU fallback)#27860
zihaomu wants to merge 2 commits into
ggml-org:masterfrom
zihaomu:feat/hip-partial-topk

Conversation

@zihaomu

@zihaomu zihaomu commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

ggml_cuda_op_top_k had a fast partial-top-k path only under CUB_TOP_K_AVAILABLE (NVIDIA CCCL >= 3.2 DeviceTopK). On ROCm/HIP that is never defined, so:

  • supports_op reported TOP_K unsupported for ne[0] > 1024, pushing it to the CPU backend (a per-token GPU<->CPU roundtrip + CPU sort), and

  • where it did run, it full-sorted every row. This collapses Qwen3-Next/qwen4exp QSA long-context decode on Strix Halo, while a same-bandwidth NVIDIA GB10 (DeviceTopK) stays flat.

  • top-k.cu: add a HIP branch using rocPRIM partial_sort_copy on packed (score,index) keys (nth_element + sort of the first k). k == ncols falls back to bitonic (rocPRIM requires middle < size).

  • ggml-cuda.cu: enable TOP_K in supports_op on HIP when k < ncols (or ncols <= 1024), and disable graph capture for graphs whose TOP_K uses the partial path (it syncs the stream), mirroring the MUL_MAT_ID guard.

Validation on gfx1151 / ROCm 7.2.1:

  • test-backend-ops -o TOP_K: 517/517 OK (incl. ncols up to 16384).
  • qwen4exp UD-IQ4_XS single-stream, graphs ON: S_TG at N_KV 640/2176/8320 = 21.5/20.7/19.3 t/s (was 21.5/8.8/8.1) -> long-context collapse eliminated.

The following is speed testing result of Qwen3.8-Flash-Next-GGUF on AMD Strix Halo 395 Chip:

N_KV before (tok/s) after (tok/s)
640 21.5 21.47
2176 8.8 20.72
8320 8.1 19.25

cc @zhangnju

Overview

Additional information

Requirements

ggml_cuda_op_top_k had a fast partial-top-k path only under CUB_TOP_K_AVAILABLE
(NVIDIA CCCL >= 3.2 DeviceTopK). On ROCm/HIP that is never defined, so:
  - supports_op reported TOP_K unsupported for ne[0] > 1024, pushing it to the CPU
    backend (a per-token GPU<->CPU roundtrip + CPU sort), and
  - where it did run, it full-sorted every row.
This collapses Qwen3-Next/qwen4exp QSA long-context decode on Strix Halo, while a
same-bandwidth NVIDIA GB10 (DeviceTopK) stays flat.

- top-k.cu: add a HIP branch using rocPRIM partial_sort_copy on packed (score,index)
  keys (nth_element + sort of the first k). k == ncols falls back to bitonic
  (rocPRIM requires middle < size).
- ggml-cuda.cu: enable TOP_K in supports_op on HIP when k < ncols (or ncols <= 1024),
  and disable graph capture for graphs whose TOP_K uses the partial path (it syncs
  the stream), mirroring the MUL_MAT_ID guard.

Validation on gfx1151 / ROCm 7.2.1:
- test-backend-ops -o TOP_K: 517/517 OK (incl. ncols up to 16384).
- qwen4exp UD-IQ4_XS single-stream, graphs ON: S_TG at N_KV 640/2176/8320 =
  21.5/20.7/19.3 t/s (was 21.5/8.8/8.1) -> long-context collapse eliminated.
@zihaomu
zihaomu requested a review from a team as a code owner August 28, 2026 08:26
@github-actions github-actions Bot added ggml changes relating to the ggml tensor library for machine learning CUDA Related to the CUDA backend labels Aug 28, 2026
@Stoney49th

Copy link
Copy Markdown

are you aware of #27466 ?

Replace the rocPRIM partial_sort_copy top-k path with hipCUB radix sort-pairs.
rocPRIM partial_sort is not stream-capture-safe, which forced disabling HIP
graphs for the decode path. hipCUB DeviceRadixSort (single row) and
DeviceSegmentedRadixSort (multi-row) are capture-safe, so single-row decode
top-k now runs inside CUDA graphs.

- top-k.cu: sort (score, index) pairs descending with radix and keep the first
  k indices of each row; drop the packed-u64 key and the per-row loop. CUB sorts
  float keys directly, so no manual bit-packing is needed.
- ggml-cuda.cu: supports_op returns true for TOP_K on ROCm for any ncols/nrows.
- ggml-cuda.cu: instantiating a HIP graph that contains a large segmented sort
  overflows the ROCm runtime's graph builder, so disable CUDA graphs only for
  the multi-row (nrows>1) top-k case; single-row decode keeps graphs.

test-backend-ops -o TOP_K: 517/517 pass; perf -o TOP_K runs clean under graphs.
@zihaomu
zihaomu marked this pull request as draft August 28, 2026 09:46
@IMbackK IMbackK closed this Aug 28, 2026
@IMbackK

IMbackK commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

closed in favor of #27466

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CUDA Related to the CUDA backend ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants