Skip to content

[AMD] Gate DFLASH non-greedy verify on the target-only kernel being registered - #33694

Merged
HaiShaw merged 2 commits into
mainfrom
amd_fix_stageb_0805
Aug 6, 2026
Merged

HaiShaw merged 2 commits into
mainfrom
amd_fix_stageb_0805

Conversation

@yctseng0211

@yctseng0211 yctseng0211 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Motivation

test/registered/spec/dflash/test_dflash.py crashes the scheduler on the AMD stage-b runner:

File "python/sglang/srt/speculative/dflash_utils.py", line 749, in compute_dflash_sampling_correct_drafts_and_bonus
    tree_speculative_sampling_target_only(
NameError: name 'tree_speculative_sampling_target_only' is not defined

#32541 added an elif is_hip(): branch that sets _DFLASH_SAMPLING_VERIFY_AVAILABLE = True but never binds tree_speculative_sampling_target_only in that branch. The kernel genuinely does not exist on ROCm: csrc/speculative/speculative_sampling.cu is listed only in the CUDA CMakeLists.txt, while setup_rocm.py compiles just eagle_utils.cu and common_extension_rocm.cc registers only verify_tree_greedy.
With the flag unconditionally True, any request with temperature > 0 entered the non-greedy verify path and hit the unbound name. That matches the CI signal: test_early_stop and test_greedy_determinism use temperature=0 and pass, while test_eos_handling uses temperature=0.1 and kills the scheduler, turning every later request into Connection refused.

Modifications

Bind the symbol in the is_hip() branch and derive _DFLASH_SAMPLING_VERIFY_AVAILABLE from whether the op is actually registered.
An ImportError guard would not work: sgl_kernel/__init__.py unconditionally re-exports the pure-Python wrapper, so the import always succeeds on ROCm and only the torch.ops.sgl_kernel.* dispatch inside it fails. Probing the op table is the reliable check, and hasattr(torch.ops.<ns>, ...) is already the idiom used in srt/utils/custom_op.py and device_communicators/cpu_communicator.py.
Scope: the diff is confined to the elif is_hip(): branch. The CUDA/MUSA branch and the else branch are byte-for-byte unchanged, so there is no path by which this can affect NVIDIA or MUSA behavior.
On ROCm the probe returns False, so dflash_worker_v2.py takes its existing greedy-argmax fallback and emits the warning that path already logs. This restores the pre-#32541 ROCm behavior and matches eagle_utils.py, which puts _is_hip in the greedy branch for the same reason.

Follow-up

This PR stops the crash; it does not add non-greedy verification on ROCm, where temperature > 0 still verifies against argmax. Two options for a real fix, both orthogonal to this PR:

  1. Build the kernel on ROCm. Add flashinfer to setup_rocm.py (setup_musa.py already does exactly this for the same file), then add the source and register the op. The open question is whether flashinfer/sampling.cuh survives hipify; encouragingly, AMD's ROCm/flashinfer port already ships a HIPified sampling.cuh.
  2. Write a Triton target-only kernel. eagle_utils.py already treats chain_speculative_sampling_triton and tree_speculative_sampling_target_only as interchangeable same-signature functions, so this needs no call-site change. DFLASH verification is a pure chain (retrieve_next_sibling is all -1), which collapses the tree walk to a linear scan. Note that chain_speculative_sampling_triton itself is not a drop-in substitute: it is classic rejection sampling, and with the all-zero draft_probs DFLASH passes, its coin * q < p test accepts unconditionally.
    Once either lands, the op probe added here flips to True on its own with no further change to this file.

Speed Tests and Profiling

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): 🚫 Run #31079763648
Latest PR Test (Extra): ❌ Run #31079763058

@yctseng0211

yctseng0211 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

[stage-b-test-1-gpu-small-amd]
https://github.com/sgl-project/sglang/actions/runs/30996436654

image

verified

@yctseng0211

Copy link
Copy Markdown
Collaborator Author

@HaiShaw this PR only stops the crash by falling back to greedy verify on ROCm; the follow-up would be getting the target-only sampling kernel into the ROCm sgl-kernel build.

@HaiShaw

HaiShaw commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@Raiden-Makoto is building the tree spec sampling kernel

@yctseng0211

Copy link
Copy Markdown
Collaborator Author

@Raiden-Makoto is building the tree spec sampling kernel

@HaiShaw since this PR gates on the op being registered rather than on is_hip(), that kernel will flip DFLASH non-greedy verify back on automatically.

@HaiShaw
HaiShaw merged commit cf79236 into main Aug 6, 2026
65 of 97 checks passed
@HaiShaw
HaiShaw deleted the amd_fix_stageb_0805 branch August 6, 2026 07:10
kfhfar pushed a commit to kfhfar/sglang that referenced this pull request Aug 7, 2026
Xia-Weiwen pushed a commit to Xia-Weiwen/sglang that referenced this pull request Aug 10, 2026
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Atituiset pushed a commit to Atituiset/sglang that referenced this pull request Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants