Fix deterministic DSV4 top-k fallback on ROCm - #34591
Open
skyguan92 wants to merge 1 commit into
Open
Conversation
ROCm installations may not register the DSV4 AOT top-k operator when their GPU target is absent from the wheel. Detect that capability and use the existing JIT v1 path instead. Make that JIT path compile through HIP and fit 64 KiB-LDS devices, select exact boundary ties by lower logical index, canonicalize output order, and recover correctly when a coarse FP16 bin exceeds the bounded LDS candidate buffer. Treat signed zeroes as equal scores on ROCm. Preserve the new runtime top-k contract for every size in (0, 1024], including non-power-of-two sizes, while leaving the CUDA implementation unchanged. Add AMD CI coverage for forced JIT fallback, repeated determinism, page sizes 1/64/256, ragged rows, boundary ties, candidate-buffer overflow, runtime top-k sizes, and the benchmarked 262144-token upper length. Signed-off-by: guanjiawei <128683929+skyguan92@users.noreply.github.com>
skyguan92
requested review from
BBuf,
DarkSharpness,
HydraQYH,
celve and
yuan-luo
as code owners
August 12, 2026 15:31
Author
|
Lint is green, including pre-commit and the sgl-kernel clang-format check. The remaining red/skip matrix is the expected label gate: the logs report Could an authorized maintainer trigger the standard CI when appropriate? The new test is registered in |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packaged ROCm AOT operator is not registered for the active target.
zeroes and coarse-bin overflow while preserving CUDA behavior.
runtime Top-K sizes, overflow, long rows and repeat determinism.
Why this is needed
On an unmodified current SGLang checkout, a native gfx1100 installation whose
wheel does not contain the DSV4 AOT operator fails twice: the public Python
path raises
AttributeError, and direct JIT v1 compilation calls theCUDA-only
cudaFuncSetAttributeandcudaFuncAttributeMaxDynamicSharedMemorySizeAPIs.The launchable ROCm path also needs deterministic handling at the exact
selection boundary. Atomic arrival order must not choose among equal scores,
signed
-0.0/+0.0must compare equally, and a coarse FP16 bin larger than thebounded LDS candidate buffer must not be silently clipped.
Contract
DeepSeek-V4 uses
index_topk=512. SGLang's current API supports runtimetopk in (0, 1024]and returns an unordered selected set; it does not promisescore-sorted output. This change canonicalizes the selected set by logical
index on ROCm, which provides a deterministic valid representation without
adding a score-order contract. CUDA code paths are unchanged.
Implementation
Physical AMD validation
Exact source pair:
The tested parent is the upstream
mainfetched immediately before thispublication gate. It is 32 commits ahead of the previous qualified parent;
none of its 247 changed paths intersects the three candidate paths. Relative
to the previous candidate
3076f2cd3, the range-diff is equal, stable patch ID8307236dde8eis unchanged, and all three candidate-file SHA-256 values areexact.
Immediately before push, upstream added one documentation-only commit that
renames the Qwen3.8 DSpark recipe. It changes two cookbook paths and has zero
intersection with this patch. The final publication source is therefore
d21eefc94ff8e95ea70ba54ddde83b35ff26d340..bd52a2dbf40c1aa287834416fa065dd7f187774f,tree
4dc2bdb2a380685e28c94f40bd7adb12a2bc272e. Its range-diff and stablepatch ID are equal to the exact physical pair above; the physical timings are
not relabelled as a run of the documentation-only replay.
Environment:
Unmodified parent blocker:
Candidate:
The test matrix covers forced AOT fallback, page sizes 1/64/256, signed
zeroes, exact boundary ties, dense-bin overflow, ragged rows, the
262144-token upper tested length, runtime
topk={1,7,257,513,777,1024}, and repeated output identity. The repositoryregistry checker passes; the file resolves to AMD
jit-kernel-unit-test-amdwith a 60-second estimate.The accepted parent and both candidate runs explicitly remove HSA/HIP/CUDA
visibility variables and select one physical card only through
ROCR_VISIBLE_DEVICES. A registry-only harness first missed the image's venvon
PATHand exited before running the checker; it is excluded. The rerun usedthe explicit interpreter, and both the repository checker and suite validation
passed.
Performance guard
This is a correctness and portability fix, not a speedup claim. On the same
source bytes in the predecessor qualification, the complete candidate versus
a launchable control was
+1.217%p50 for page output and+1.153%for pageplus raw output. A matched ModelScope DeepSeek-V4-Flash-0731 TP8 A-B-A run
completed 27/27 S3072 requests; the full 21-layer Indexer bucket changed by
+0.162186%and+0.179978%in the two candidate brackets, inside thepredeclared 1% guard. That model gate used the same disclosed cached-BF16
lower shim on both sides and is predecessor/source-equivalent evidence, not an
exact-current native-FP8 throughput claim.
Related work
seq_lenson pre-rewrite history. Itscontract is independent; a semantic v1 compatibility port passed beside
this patch, but that scope is intentionally not included here.
fallback, HIP attribute or deterministic selected-set behavior.
SM90/SM100. It does not repair the existing gfx1100 DSV4 Top-K v1 path,
missing-AOT fallback, HIP attributes, 64-KiB-LDS behavior or AMD registered
coverage.
Test plan
git diff --checkCI States
Latest PR Test (Base): ❌ Run #31612796955
Latest PR Test (Extra): ❌ Run #31612796402