Repository navigation
[Fix] TensorMatcher: keep symbol device/dtype options; use kDLGPU aliases - #47300
Merged
Merged
Conversation
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
DarkSharpness
requested review from
BBuf,
Fridge003,
HaiShaw,
HydraQYH,
Ying1123,
celve,
jybsuper,
lifuhuang,
yuan-luo and
yushengsu-thu
as code owners
October 29, 2026 17:17
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
DarkSharpness
force-pushed
the
fix-tensor-matcher-options
branch
from
October 29, 2026 21:12
49e237a to
878c941
Compare
…ases `with_device(sym)` / `with_dtype(sym)` with an empty type pack called `set_options<>()`, which replaced the symbol's options with an empty list (= anything allowed). Launchers that did `device.set_options<kDLCUDA>()` followed by `.with_device(device)` therefore accepted CPU tensors, and `nvlink_comm` accepted any dtype. - Only override options when the pack is non-empty. - `set_options` re-checks an already-bound value, so a runtime `DLDevice` / `DLDataType` passed with a type list is checked too. - Use `kDLGPU` / `kDLGPUHost` instead of hard-coded `kDLCUDA` / `kDLCUDAHost` in JIT launchers and headers. - Add a JIT regression test for both paths. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Files with unguarded PTX, cute/cutlass/cub/flashinfer/TRT-LLM includes, or an NVIDIA arch in the name cannot build on ROCm, so they keep kDLCUDA. The skill doc says when kDLCUDA is the right choice. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
DarkSharpness
force-pushed
the
fix-tensor-matcher-options
branch
from
October 29, 2026 22:13
878c941 to
2f73cbb
Compare
BBuf
approved these changes
Oct 30, 2026
kevin-mii
added a commit
to kevin-mii/sglang
that referenced
this pull request
Nov 1, 2026
Main's TensorMatcher (sgl-project#47300) now holds a bound device to its options and names the platform GPU kDLGPU; this ROCm-only entry point still asked for kDLCUDA, so every all-reduce on a rocm device failed the match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kevin-mii
added a commit
to kevin-mii/sglang
that referenced
this pull request
Nov 1, 2026
Main's TensorMatcher (sgl-project#47300) now holds a bound device to its options and names the platform GPU kDLGPU; this ROCm-only entry point still asked for kDLCUDA, so every all-reduce on a rocm device failed the match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
kevin-mii
added a commit
to kevin-mii/sglang
that referenced
this pull request
Nov 1, 2026
Main's TensorMatcher (sgl-project#47300) now holds a bound device to its options and names the platform GPU kDLGPU; this ROCm-only entry point still asked for kDLCUDA, so every all-reduce on a rocm device failed the match. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4 of 5 tasks
Yuxingwang-intel
pushed a commit
to Yuxingwang-intel/sglang
that referenced
this pull request
Nov 6, 2026
…ases (sgl-project#47300) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
TensorMatcher::with_device<Codes...>(sym)andwith_dtype<Ts...>(sym)calledset_optionseven with an empty pack, which replaced the symbol's options with an empty list (= anything allowed). The common launcher idiomdevice.set_options<kDLCUDA>(); TensorMatcher(...).with_device(device).verify(x);therefore never checked the device type, so a CPU tensor passed. About 200 of 260SymbolicDevicedeclarations underpython/sglang/kernels/jit/csrc/used this idiom. The same bug affected the dtype restriction indistributed/nvlink_comm.cuh.Introduced in #14570, which added the
set_optionscall to the symbol overloads; before that (#13453) they left the symbol's options untouched, so every launcher written with this idiom since has had an unchecked device type.Modifications
tensor.h:with_device(sym)/with_dtype(sym)only override options when the pack is non-empty.set_optionsnow re-checks a value that is already bound, so a runtimeDLDevice/DLDataTypepassed together with a type list (e.g..with_device<kDLGPU>(x.device())) is checked too; that path was also unchecked before.kDLCUDA/kDLCUDAHostwithkDLGPU/kDLGPUHostin JIT launchers and headers (jit/csrc,jit/include,kda_kernels/csrc), so the check is correct on ROCm.moe/paged_experts.cuhneededusing sglang::kDLGPU;because it only importssglang::host.kDLCUDAbecause they are CUDA-only and don't includesgl_kernel/utils.cuh:jit/csrc/moe/tvm_ffi_utils.h(cudaGetDevice+ CUDA stream helper), the vendoredops/lora/moe/trtllm_lora_temp/data/csrc/trtllm_fused_moe_kernel_launcher.cu, and the device-name table intensor.h.add-jit-kernelskill examples updated tokDLGPU, plus a note that options set on a symbol stay in force.test/registered/unit/kernels/test_tensor_matcher.py: JIT-compiles a small launcher and checks that a CPU tensor or a wrong dtype is rejected through both the symbol path and the runtime-value path. All 4 cases fail on the old header.Fallout
No launcher started rejecting its inputs: no test anywhere hit
not in the allowed options, so no call site or declared option needed changing.Note:
moe/expert_specialization/es_sm100_mxfp8_blockscaled_{group_quant,moe_group_gemm}.cuhare not referenced by any JIT loader and already fail to compile on main (hostis not visible there). They were only touched by the rename.Accuracy / tests
test/registered/kernels/ops/**andtest/registered/unit/kernels/**on 1x B200. Every failure reproduces on main with identical counts (paged_mqa_metadata,q8kv8_sparse_prefill_backend,tilelang_dsa_zero_rope,model_fast_paths, diffusiontest_rope,hy4_decode_kernels,lanczos,moe_topk_softmax,per_token_group_quant_8bit_v2). The rest are multi-GPU or ROCm-only tests that can't run on a single CUDA GPU.all_reduce_residual_norm,deterministic_reduce_scatter,fp4_allgather_moe_reduction,tp_qknorm,custom_all_reduce,symm_mem_all_gatherpass.ar_fusionandbeam_all_reduceneed 8 GPUs (left to CI).Follow-up (not in this PR)
Hand-written checks that should become matcher expressions (
RuntimeCheck(t.dim() == ...), stride / contiguity / alignment checks, raw dtype.code/.bits,TVM_FFI_ICHECK) remain in about 90 files underjit/csrc, e.g.distributed/custom_all_reduce.cuh,gemm/fp8_blockwise/fp8_blockwise_scaled_mm_sm120.cuh,elementwise/fused_qknorm_rope.cuh,inkling/inkling_all_reduce.cuh,deepseek_v4/hash_topk.cuh.🤖 Generated with Claude Code
CI States
Latest PR Test (Base): 🚫 Run #40964193122
Latest PR Test (Extra): ❌ Run #40964193139
Latest PR Test (AMD ROCm 10): 🚫 Run #40964192940