[AMD] Enable JIT staged HiCache write-back and fix CPU-index crash - #28534
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces ROCm/HIP support for the JIT HiCache kernels by guarding NVIDIA-specific PTX instructions and utilizing non-temporal builtins on ROCm. It also updates the cache controller and host memory pool to enable and check JIT kernel availability on HIP platforms, preventing crashes during write-back. The review feedback highlights critical performance and correctness improvements: first, using native Clang vector types (ext_vector_type) for uint2 and uint4 loads and stores on ROCm to ensure proper compiler vectorization; second, handling cases where self.mem_pool_host is a HostPoolGroup by checking its underlying anchor pool for JIT support so that the optimization is not silently disabled.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ( | ||
| self.io_backend == "kernel" | ||
| and self.mem_pool_host.layout == "page_first" | ||
| and getattr(self.mem_pool_host, "can_use_jit", False) | ||
| ): |
There was a problem hiding this comment.
When self.mem_pool_host is a HostPoolGroup (which is used for grouped pools like DSA/MLA models), it does not directly expose the can_use_jit attribute. As a result, getattr(self.mem_pool_host, "can_use_jit", False) will return False, silently disabling the JIT staged write-back optimization for these models.
We should also check the can_use_jit attribute of the underlying anchor pool if self.mem_pool_host is a HostPoolGroup.
| if ( | |
| self.io_backend == "kernel" | |
| and self.mem_pool_host.layout == "page_first" | |
| and getattr(self.mem_pool_host, "can_use_jit", False) | |
| ): | |
| if ( | |
| self.io_backend == "kernel" | |
| and self.mem_pool_host.layout == "page_first" | |
| and ( | |
| getattr(self.mem_pool_host, "can_use_jit", False) | |
| or ( | |
| hasattr(self.mem_pool_host, "anchor_entry") | |
| and getattr(self.mem_pool_host.anchor_entry.host_pool, "can_use_jit", False) | |
| ) | |
| ) | |
| ): |
eeec323 to
09d02f9
Compare
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
On ROCm, page_first + kernel HiCache write-back crashed on the first
prefill with:
RuntimeError: Destination indices must be a CUDA tensor
Root cause: HiCacheController.start_writing() keeps host_indices on the
CPU for the kernel io-backend + page_first layout, assuming the staged
JIT write-back kernel (which stages through device memory and accepts a
CPU destination index) will consume them. That JIT path is gated behind
`_is_cuda`, so on ROCm it is disabled and the code falls back to the
plain `transfer_kv_all_layer_mla_lf_pf` C++ kernel, whose launcher
asserts `dst_indices.is_cuda()`. CPU host_indices -> assert -> all TP
scheduler ranks crash -> prefill dies.
Fixes:
- hicache.cuh: guard the NVIDIA-only PTX `ld/st.global.L1::no_allocate`
helpers (load_nc/store_nc) behind `#ifndef USE_ROCM` and provide ROCm
equivalents using non-temporal loads/stores, so the JIT HiCache module
also builds with hipcc. The staged write-back kernel already has a
USE_ROCM path. Verified the module compiles and loads on gfx942/ROCm 7.2.
- memory_pool_host.py: allow `can_use_jit` on HIP, not only CUDA, so ROCm
uses the same staged write-back path as CUDA.
- cache_controller.py: only keep host_indices on CPU when the staged JIT
kernel is actually available (`can_use_jit`); otherwise move them to the
device as before. This makes the kernel io-backend correct on any
backend where the JIT kernel is unavailable, independent of the change
above.
The staged write-back kernel's TensorMatcher checks hard-coded kDLCUDA / kDLCUDAHost, so on ROCm the device-resident tensors (staging, layer ptrs, page indices) and pinned host buffers fail verification with "Tensor match failed ... device=rocm:N" at staged_write_back.cuh. Accept kDLROCM for device tensors and kDLROCMHost for host tensors, mirroring the kDLCUDA/kDLROCM pattern already used by the other JIT kernels (clamp_position, kvcache, resolve_future_token_ids).
…hers The non-staged HiCache JIT kernels (load host->device, write store) in hicache.cuh have the same CUDA-only TensorMatcher device checks as the staged kernel. These are exercised on prefix-cache hits (load path), so warmup (write-only) passed but profiling with resumed sessions crashed prefill with "Tensor match failed ... device=rocm:N at hicache.cuh". Add kDLROCM (device) / kDLROCMHost (pinned host) to all device matchers, same as the staged_write_back.cuh fix.
Express the ROCm load_nc/store_nc paths as a single __builtin_nontemporal_
{load,store} over Clang ext_vector_type(2/4) instead of N independent 32-bit
ops. This makes the vectorized global_{load,store}_dwordx{2,4} with the
nontemporal hint deterministic rather than relying on the LoadStoreVectorizer
to merge per-scalar accesses. Use __builtin_bit_cast to convert between uintN
and the native vector type to avoid strict-aliasing UB.
8485afd to
21243ba
Compare
|
/tag-and-rerun-ci |
|
@amd-bot ci-status |
CI Status for PR #28534Merge verdict: Likely safe to merge. All 14 executed CI failures are pre-existing perf-threshold flakes, infra library-load errors, or unrelated tests — none are caused by this PR's changes. PR CI is complete (185/185 checks finished, nothing pending/fast-fail-skipped). The changed dispatch logic + CPU-index fix is exercised and green on the NVIDIA CPU suite; however, the AMD-hardware JIT-kernel enablement (the PR's headline feature) is not directly exercised by any AMD PR-CI test. Warning Coverage is partial. The changed Python ( Changed files: Executed CI failure attribution: AMD: 7 failures (0 related) · Others (NVIDIA/NPU/XPU): 7 failures (0 related) · 6 aggregator AMD Executed Failures
Other Executed Failures
Details / what to do before merge
Generated by amd-bot using Claude Code CLI |
HaiShaw
left a comment
There was a problem hiding this comment.
Can we add an unit test case file as well?
| .with_strides({N, 1}) | ||
| .with_dtype(cache_dtype) | ||
| .with_device<kDLCUDA, kDLCUDAHost, kDLCPU>() | ||
| .with_device<kDLCUDA, kDLROCM, kDLCUDAHost, kDLROCMHost, kDLCPU>() |
There was a problem hiding this comment.
Can we use KDLGPU instead?
There was a problem hiding this comment.
Addressed your comments and added unit tests. PTAL.
|
@amd-bot ci-status |
CI Status for PR #28534Merge verdict: No executed CI failure is attributable to this PR — all 14 real failures are pre-existing perf regressions, infra/runner problems (VRAM-cleanup, missing Caution This PR's headline change (enabling Changed files: What IS covered (green): CUDA Executed CI failure attribution: AMD: 7 failures (0 related) · Others: 7 failures (0 related). 6 additional AMD Executed Failures
Other Executed Failures
Details / what to do before merge
Generated by amd-bot using Claude Code CLI |
…ck unit test Address review feedback on the HiCache JIT device-type matchers: - Replace the explicit `kDLCUDA, kDLROCM` / `kDLCUDAHost, kDLROCMHost` enumerations in hicache.cuh and staged_write_back.cuh with the platform-conditional `kDLGPU` / `kDLGPUHost` aliases, matching the convention already used by the other JIT kernels. Add the missing `kDLGPUHost` alias next to `kDLGPU` in utils.cuh so the host side is symmetric. - Add test/registered/jit/test_hicache_page_first_write_back.py covering the page_first + `kernel` staged write-back (D2H) and load (H2D) roundtrip for MHA and MLA across page counts around the staging capacity. Unlike test_hicache.py (CUDA-only), this file is also registered for the AMD PR-CI kernel suite so the ROCm/HIP build and execution of the modified kernels are validated on AMD hardware. Verified passing on MI355X (gfx950, ROCm 7.2).
… API The new test imported ALLOC_MEMORY_FUNCS / alloc_with_pin_memory from memory_pool_host and asserted host_pool.can_use_jit, which broke on the merged main: those helpers now live in sglang.srt.mem_cache.pool_host.common and the JIT flag was split into can_use_jit (load/transfer) and can_use_write_back_jit (staged write-back). Align the imports and assert can_use_write_back_jit for the write-back path, mirroring test_hicache.py. Verified passing on MI355X (gfx950, ROCm 7.2).
|
@amd-bot ci-status |
CI Status for PR #28534Merge verdict: No executed CI failure is attributable to this PR — the changed hicache code IS exercised and green (new Warning Changed paths are covered by Changed files: Executed CI failure attribution: AMD: 3 failures (0 related) · Others: 11 failures (0 related) · plus 3 fast-fail cascade AMD Executed Failures
Other Executed Failures
Details / what to do before merge
Generated by amd-bot using Claude Code CLI |
Resolve conflict in memory_pool_host.py from main's mem_cache refactor (MHA host pool moved to pool_host/mha.py; new pools added). Reapply the PR's HIP enablement so MHA and MLA host pools keep can_use_jit / can_use_write_back_jit on ROCm: - memory_pool_host.py: MLATokenToKVPoolHost (_is_cuda -> _is_cuda or _is_hip) - pool_host/mha.py: MHATokenToKVPoolHost (_is_cuda -> _is_cuda or _is_hip) Newly-added pools (DeepSeekV4/Mamba/DSA) are left CUDA-only (out of this PR's scope). Also update the new unit test import for MHATokenToKVPoolHost's new location (pool_host.mha). Verified: HiCache JIT tests pass on MI355X (ROCm 7.2).
…gl-project#28534) Co-authored-by: Duyi-Wang <duyi.wang@amd.com>
On ROCm,
page_first+kernelHiCache write-back crashes on the first prefill withRuntimeError: Destination indices must be a CUDA tensor. This PR fixes the underlying cause so ROCm runs the samepage_first+kernelJIT staged write-back path as CUDA, instead of the #28473layer_firstfallback.Root cause
HiCacheController.start_writing()keepshost_indiceson the CPU for thekernelio-backend +page_firstlayout, assuming the staged JIT write-back kernel (which stages through device memory and accepts a CPU destination index) will consume them. That JIT path is gated behind_is_cuda, so on ROCm it is disabled and the code falls back to the plaintransfer_kv_all_layer_mla_lf_pfC++ kernel, whose launcher assertsdst_indices.is_cuda(). CPUhost_indices-> assert -> all TP scheduler ranks crash -> prefill dies.Why fix the cause instead of the #28473
layer_firstfallback#28473 works around the same crash by forcing
hicache_mem_layout = "layer_first"on ROCm wheneverpage_first+kernelis requested. That keeps CI green, but it:layer_firstpath and never benefits from the staged kernel that CUDA uses;page_first+kernelROCm path broken.This PR fixes the underlying cause so ROCm keeps the same path as CUDA: platform parity, retained staged write-back bandwidth, vectorized non-temporal device transfers, and no second code path to maintain.
Once this lands, #28473 should be reverted — its ROCm
layer_firstfallback inServerArgs._resolve_layout_io_compatibility()is no longer needed (and would otherwise keep short-circuiting ROCm away from the now-correctpage_first+kernelJIT path).Modifications
hicache.cuh: guard the NVIDIA-only PTXld/st.global.L1::no_allocatehelpers (load_nc/store_nc) behind#ifndef USE_ROCMand provide ROCm equivalents using non-temporal loads/stores, so the JIT HiCache module also builds with hipcc. The ROCm paths use a single__builtin_nontemporal_{load,store}over Clangext_vector_type(2/4)(with__builtin_bit_cast) so the vectorizedglobal_{load,store}_dwordx{2,4}non-temporal ops are deterministic instead of relying on the LoadStoreVectorizer.memory_pool_host.py: allowcan_use_jiton HIP, not only CUDA, so ROCm uses the same staged write-back path as CUDA.cache_controller.py: only keephost_indiceson CPU when the staged JIT kernel is actually available (can_use_jit); otherwise move them to the device. This makes thekernelio-backend correct on any backend where the JIT kernel is unavailable.staged_write_back.cuh/hicache.cuhdevice matchers: acceptkDLROCM(device) /kDLROCMHost(pinned host) alongsidekDLCUDA/kDLCUDAHost, mirroring the other JIT kernels (clamp_position,kvcache,resolve_future_token_ids).Reproduction
2P1D ROCm deployment with
page_first+kernelHiCache write-back (--hicache-io-backenddefaults tokernel, the JIT staged write-back path this PR fixes). Core launch commands (TP=8, Kimi-K2.6-MXFP4):Prefill
SGLANG_USE_AITER=1 SGLANG_AITER_MLA_PERSIST=1 AITER_MXFP4_MOE_SF=1 \ python3 -m sglang.launch_server \ --model-path /models/amd/Kimi-K2.6-MXFP4 \ --served-model-name Kimi-K2.6-MXFP4 \ --tool-call-parser kimi_k2 --reasoning-parser kimi_k2 \ --chat-template /models/amd/Kimi-K2.6-MXFP4/chat_template.jinja \ --tp-size 8 --page-size 64 \ --context-length 262144 --kv-cache-dtype bf16 \ --attention-backend aiter \ --mem-fraction-static 0.8 --max-running-requests 128 \ --chunked-prefill-size 16384 \ --cuda-graph-bs $(seq 1 128) --cuda-graph-max-bs 128 \ --trust-remote-code \ --disaggregation-mode prefill \ --disaggregation-transfer-backend mori \ --disaggregation-bootstrap-port 8998 \ --disaggregation-ib-device ionic_0,ionic_1,ionic_2,ionic_3,ionic_4,ionic_5,ionic_6,ionic_7 \ --enable-hierarchical-cache --hicache-size 192 \ --hicache-mem-layout page_first --hicache-write-policy write_through \ --enable-metrics --enable-cache-report \ --host 0.0.0.0 --port 30020Decode
SGLANG_USE_AITER=1 SGLANG_AITER_MLA_PERSIST=1 AITER_MXFP4_MOE_SF=1 \ python3 -m sglang.launch_server \ --model-path /models/amd/Kimi-K2.6-MXFP4 \ --served-model-name Kimi-K2.6-MXFP4 \ --tool-call-parser kimi_k2 --reasoning-parser kimi_k2 \ --chat-template /models/amd/Kimi-K2.6-MXFP4/chat_template.jinja \ --tp-size 8 --page-size 64 \ --context-length 262144 --kv-cache-dtype bf16 \ --attention-backend aiter \ --mem-fraction-static 0.85 --max-running-requests 128 \ --chunked-prefill-size 8192 \ --cuda-graph-bs $(seq 1 128) --cuda-graph-max-bs 128 \ --num-continuous-decode-steps 4 \ --trust-remote-code \ --disaggregation-mode decode \ --disaggregation-transfer-backend mori \ --disaggregation-bootstrap-port 19100 \ --disaggregation-ib-device ionic_0,ionic_1,ionic_2,ionic_3,ionic_4,ionic_5,ionic_6,ionic_7 \ --enable-metrics --enable-cache-report \ --host 0.0.0.0 --port 30030Router (native PD-disaggregation router)
Then send multi-turn / prefix-reusing traffic at the router (
:8100). Before this PR, on ROCm the first prefill HiCache write-back crashes withRuntimeError: Destination indices must be a CUDA tensor, and the first prefix-cache hit crashes withTensor match failed ... device=rocm:N. With this PR both paths run cleanly.Accuracy / functional tests
load_nc/store_ncemit singleglobal_load_dwordx2/x4andglobal_store_dwordx2/x4with the non-temporal (nt) flag.page_first+kernel,write_through) running the AgentX v0.3 agentic trace replay at concurrency 64: warmup + 900s profiling complete (328 requests, 0 errors) with noDestination indices must be a CUDA tensorand noTensor match failed ... device=rocm:Ncrashes (both crashed pre-fix on the first write-back / first prefix-cache-hit load).Speed tests and profiling
To check that the staged write-back path is worth keeping on ROCm (rather than degrading to
layer_firstper #28473), I ran the equivalent of the #21631 write-back micro-benchmark on AMD MI355X / gfx950, ROCm 7.2, comparing the installedsgl_kernelLF->PF kernels against the JIT staged LF->PF kernels on the same host-destination write-back workload.Setup mirrors #21631:
dtype=bf16,page_size=64,batch_pages=64,total_pages=128, timing viatriton.testing.do_bench(warmup=5, rep=25). Each kernel is fed indices in the residency it requires (baseline*_lf_pf: devicedst_indices; staged: pinned-hostdst_indices); correctness is verified withtorch.testing.assert_closefor every row before timing. MHA bandwidth countsK + V; MLA counts a single buffer.speedup = staged_jit / sgl_kernel_lf_pf. ROCm has nocudaMemcpyBatchAsyncequivalent, sostaged_write_back.cuhuses the non-batch ROCm fallback (device relayout into staging + per-page async H2D copies).Per-shape write-back microbenchmark (MHA / MLA)
MHA (
batch_pages=64)sgl_kernel *_lf_pfGiB/sjit *_staged_lf_pfGiB/sMLA (
batch_pages=64)sgl_kernel *_lf_pfGiB/sjit *_staged_lf_pfGiB/sTakeaway: on gfx950 the
*_lf_pfkernels top out around 15-30 GiB/s, while the staged JIT path reaches ~25-50 GiB/s (close to the same peak CUDA achieves). That is a 1.21x-2.85x speedup for MHA and 1.07x-1.86x for MLA. The gap is larger than on CUDA (where #21631 reported roughly parity, ~0.87-1.25x, because the CUDA*_lf_pfbaseline already runs near peak), so keeping ROCm on the stagedpage_first+kernelpath is a clear win over thelayer_firstfallback.CI States
Latest PR Test (Base): ⏳ Run #28997089073
Latest PR Test (Extra): ⏳ Run #28997088947