Repository navigation
[AMD][Bugfix] Fix vattn_asm HIP error 709 under CUDA graph capture on ROCm 10 - #39513
Merged
Merged
Conversation
ROCm 10 wheels load HIP from _rocm_sdk_core while LD_LIBRARY_PATH points at a second _rocm_sdk_devel copy. Opening the soname created a second HIP runtime, so hipModuleLaunchKernel on a torch side stream or HIP graph returned hipErrorContextIsDestroyed (709). Co-authored-by: Cursor <cursoragent@cursor.com>
chuyeh
marked this pull request as ready for review
September 15, 2026 04:09
chuyeh
requested review from
BBuf,
DarkSharpness,
HaiShaw,
HydraQYH,
celve and
yuan-luo
as code owners
September 15, 2026 04:09
Reuse the helper sgl-project#30870 added for the same class of failure on CUDA instead of parsing /proc/self/maps a second time, and drop the _rocm_sdk_core preference: only torch's copy is ever mapped, so that branch never ran. Move the regression test next to the other gfx950 asm kernel tests, where the existing fixtures build the inputs, and revert the Triton wrapper's test file. Co-authored-by: Cursor <cursoragent@cursor.com>
The two copies do not differ by SONAME: torch maps _rocm_sdk_core's libamdhip64.so.7, which an unversioned CDLL request never matches, so the loader takes _rocm_sdk_devel's off LD_LIBRARY_PATH. That is also why the single-runtime 7.2.0 and 7.2.4 images were never affected. Co-authored-by: Cursor <cursoragent@cursor.com>
Collaborator
|
@zijiecode Can you help review this PR? |
Collaborator
|
Protection for ROCm 10, LGTM. |
yichiche
approved these changes
Sep 16, 2026
yichiche
left a comment
Collaborator
There was a problem hiding this comment.
Fixed compatibility issue to support verify asm kernel on ROCm10. LGTM.
HaiShaw
approved these changes
Sep 17, 2026
michaelzhang-ai
added a commit
that referenced
this pull request
Sep 17, 2026
#39513 fixed the ROCm 10 split-SDK failure by binding the HIP runtime already mapped by torch, but falling back to an unversioned SONAME can still map an unidentified second runtime when lookup is ambiguous. Require exactly one mapped libamdhip64 path, bind that absolute path, and validate the binding before backend routing. This keeps GQA-8 requests on the existing fallback path if the runtime cannot be identified safely. Add regression coverage for duplicate mappings and assert that the live launcher uses the process's sole mapped HIP runtime. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
michaelzhang-ai
added a commit
that referenced
this pull request
Sep 18, 2026
#39513 fixed the ROCm 10 split-SDK failure by binding the HIP runtime already mapped by torch, but falling back to an unversioned SONAME can still map an unidentified second runtime when lookup is ambiguous. Require exactly one mapped libamdhip64 path, bind that absolute path, and validate the binding before backend routing. This keeps GQA-8 requests on the existing fallback path if the runtime cannot be identified safely. Add regression coverage for duplicate mappings and assert that the live launcher uses the process's sole mapped HIP runtime. Co-authored-by: Cursor Agent <cursoragent@cursor.com>
zijiecode
added a commit
to zijiecode/sglang
that referenced
this pull request
Sep 20, 2026
…gl-project#39513) ROCm 10 images ship a second libamdhip64 in _rocm_sdk_devel; the unversioned CDLL("libamdhip64.so") picked it and every launch on a torch stream failed with 709 (context is destroyed). Initialize CUDA first and bind to torch's copy via find_loaded_library(). Unit test passes on rocm720 and rocm10 MI35x images.
4 of 5 tasks
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.
Motivation
#37465 puts EAGLE verify / draft-extend / q_len-1 decode on the gfx950
vattn_asmkernel, launched through ctypeshipModuleLaunchKernelon the current torch stream. On ROCm 10 images,SGLANG_ASM_VERIFY_ATTN=1fails during EAGLE verify HIP-graph capture:hipModuleLoadsucceeded, so the kernel reports itself active, and the default stream (handle 0) still works. Only a torch stream fails — graph capture included, which is the path decode takes.Root cause
ROCm 10 ships the HIP runtime (
_rocm_sdk_core) and the toolchain (_rocm_sdk_devel) as separate wheels. Torch maps core'slibamdhip64.so.7, which the unversionedCDLL("libamdhip64.so")does not match, so the loader takes devel's copy offLD_LIBRARY_PATHas a second HIP runtime: the module loads in devel while the stream belongs to core.Unpatched, on MI355X:
CDLL("libamdhip64.so")bindsrocm720/opt/rocmsymlinkrocm724torch/lib/libamdhip64.sorocm10So the trigger is the SDK split, not torch 2.11 —
rocm724has torch 2.11 and one runtime — and the change is a no-op wherever the soname already resolves to torch's object. Same failure mode #30870 fixed for CUDA, where TileLang'slibcudart_stub.socould win over the real runtime.Modifications
vattn_asm_gfx950/__init__.py: initialize CUDA so torch's HIP is mapped, then bind ctypes to it with the existingfind_loaded_library()helper from Avoid TileLang CUDA runtime pollution #30870 rather than to the SONAME. Ten lines in one function; no new private API, no second/proc/self/mapsparser.No disambiguation between the two ROCm 10 copies is included, because none is needed: only torch's copy is ever mapped. After
import torch, after CUDA init, and after importingaiter,unified_attention_3d_mtpandaiter_backend,/proc/self/mapsholds exactly onelibamdhip64:The devel copy only enters the process if something explicitly
dlopens it, which is precisely the bug being removed. Should a future image map both,find_loaded_library()is the single place to teach the preference, for CUDA and HIP at once.Accuracy Tests
test/registered/amd/test_vattn_segplan_mi35x.py::TestVattnSegPlan::test_launches_on_a_side_stream_and_under_graph_capture(stage-b-test-1-gpu-small-amd-mi35x) launches the kernel on a torch side stream, then captures and replays it in a HIP graph, checking both against the file's existing fp32 reference. It lives with the other gfx950 asm kernel tests and reuses their fixtures.Verified on MI355X (gfx950):
v0.5.18-rocm10-mi35x-20260902FAILED ... 709 (context is destroyed)3 passed, 16 subtests passedv0.5.19-rocm720-mi35x-202609133 passed, 16 subtests passedv0.5.18-rocm724-mi35x-202608253 passed, 16 subtests passedThe ROCm 10 unpatched column is a negative control: reverting only
_hip_lib()toCDLL("libamdhip64.so")makes the new test fail with exactly the reported 709, so the test guards the regression rather than merely passing. With the fix, ctypes binds/opt/venv/.../_rocm_sdk_core/lib/libamdhip64.so.7, the same runtime torch uses.Speed Tests and Profiling
No speed change: one extra library-path lookup, once per process, at kernel load. This restores the assembly verify path under ROCm 10 HIP-graph capture; the kernel and launch geometry are untouched.
Checklist
Review and Merge Process
/tag-and-rerun-ci,/tag-run-ci-label,/rerun-failed-ciCI States
Latest PR Test (Base): 🚫 Run #35178338784
Latest PR Test (Extra): ❌ Run #35178338599
Latest PR Test (AMD ROCm 10): ❌ Run #35178338783