[#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 - #12705
Conversation
📝 WalkthroughWalkthroughUpdated CUDA-core enablement logic in the Linear module to recognize additional GPU architectures. The condition now enables CUDA cores for both Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/_torch/modules/linear.py (1)
2572-2574: Centralize the CUDA-core capability allowlist to avoid cross-file drift.This SM121 enablement is correct, but the same capability check is duplicated elsewhere and is already inconsistent (
tensorrt_llm/_torch/auto_deploy/custom_ops/quantization/quant.py:109-120still excludes(12,1)). Consider moving the allowlist into a shared helper/constant and reusing it in both places.Suggested direction
+# e.g., in a shared module +CUDA_CORE_CAPABILITIES = {(8, 9), (12, 0), (12, 1)} + # in Linear.__init__ self.enable_cuda_core = False if torch.cuda.is_available(): capability = torch.cuda.get_device_capability(torch.device('cuda:0')) - self.enable_cuda_core = (capability[0] == 8 and capability[1] == 9) \ - or (capability[0] == 12 and capability[1] in (0, 1)) + self.enable_cuda_core = capability in CUDA_CORE_CAPABILITIESApply the same shared constant/helper in
tensorrt_llm/_torch/auto_deploy/custom_ops/quantization/quant.pyto keep behavior aligned.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tensorrt_llm/_torch/modules/linear.py` around lines 2572 - 2574, Centralize the CUDA core allowlist by adding a shared constant or helper (e.g., CUDA_CORE_ALLOWLIST or is_cuda_core_supported(capability)) and replace the inline capability check in LinearModule (where enable_cuda_core is set using capability) with a call/reference to that helper; then update the quantization code in tensorrt_llm/_torch/auto_deploy/custom_ops/quantization/quant.py to use the same helper so (12,1) is included consistently across both places. Ensure the shared symbol explicitly contains the tuples {(8,9), (12,0), (12,1)} (or logic that yields the same) and update references in the LinearModule (enable_cuda_core) and the quant module to use that single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tensorrt_llm/_torch/modules/linear.py`:
- Around line 2572-2574: Centralize the CUDA core allowlist by adding a shared
constant or helper (e.g., CUDA_CORE_ALLOWLIST or
is_cuda_core_supported(capability)) and replace the inline capability check in
LinearModule (where enable_cuda_core is set using capability) with a
call/reference to that helper; then update the quantization code in
tensorrt_llm/_torch/auto_deploy/custom_ops/quantization/quant.py to use the same
helper so (12,1) is included consistently across both places. Ensure the shared
symbol explicitly contains the tuples {(8,9), (12,0), (12,1)} (or logic that
yields the same) and update references in the LinearModule (enable_cuda_core)
and the quant module to use that single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e59cfc02-a160-49f6-a50f-c9d5c7c47013
📒 Files selected for processing (1)
tensorrt_llm/_torch/modules/linear.py
The enable_cuda_core check in NVFP4Linear only matches SM89 and SM120 but not SM121 (DGX Spark GB10). CudaCoreNVFP4Runner.MIN_SM_VERSION is 100 so SM121 qualifies, but the linear module early-exit optimization bypasses the autotuner for small M dimensions (M <= 8) and this path was dead on SM121. Add capability (12, 1) to the check. Signed-off-by: Mihai Chiorean <mihai.v.chiorean@gmail.com>
81ea2be to
ee80a41
Compare
Co-authored-by: Gal Hubara-Agam <96368689+galagam@users.noreply.github.com> Signed-off-by: Mihai Chiorean <mihai.v.chiorean@gmail.com>
|
@galagam circling back here, does this look good? |
Use 'capability in ((8, 9), (12, 0), (12, 1))' to match the equivalent condition already in quant.py, rather than the two-step boolean form. Signed-off-by: Mihai Chiorean <mihai.v.chiorean@gmail.com>
|
@galagam — friendly bump on this one. Your suggestions from the 2026-04-23 review are in on commit
@HuiGao-NV has already approved. If the current form looks good, would you mind dismissing the changes-requested review (or re-approving) so this can move forward? Happy to make further adjustments if anything else needs to change. |
|
@galagam @HuiGao-NV friendly ping. any chance you could look at the changes and start CI? I believe I've addressed the change request. |
|
/bot run |
|
@NVIDIA/trt-llm-runtime-devs please help review |
|
PR_Github #61726 [ run ] triggered by Bot. Commit: |
|
PR_Github #61726 [ run ] completed with state |
…nnahz/dep-1083-port-flashinfer-stable-va-lifecycle-for-native-all-reduce * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (54 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
…nnahz/dep-1082-shared-mnnvl-moe-lifecycle * 'main' of https://github.com/NVIDIA/TensorRT-LLM: (142 commits) [NVIDIA#15673][fix] Enable CUDA core fast path for SM89/SM120/SM121 (NVIDIA#12705) [None][test] Adjust timeout cases in QA perf test (NVIDIA#16894) [https://nvbugs/6157892][fix] Mistral format refactor (NVIDIA#15123) [None][feat] Add kimi_k2/glm_5 grouped routing and fused router to bench_moe (NVIDIA#16830) [https://nvbugs/6501376][fix] Test-only fix — drop the `if hidden_size % 2 != 0: with pytest.raises(...)`… (NVIDIA#16844) [TRTLLM-13642][feat] Add perf sanity tests for Llama-3.1-8B and Gemma-3-1B and verify cache transceiver V2 support (NVIDIA#16355) [https://nvbugs/6433376][fix] Update the Dense test to mirror the MoE sibling — assert `bfloat16` under… (NVIDIA#16203) [None][fix] Resolve NVFP4 mixed-precision base layers for the DSpark draft (NVIDIA#16831) [https://nvbugs/6479324][test] Remove waiver for fixed qwen3_5_4b_fp8_stress disaggregated stress test (NVIDIA#16878) [https://nvbugs/6507109][infra] Split slow DGX B300 attention unit tests (NVIDIA#16838) [None][infra] Waive 21 failed cases for main in post-merge 2862 (NVIDIA#16882) [None][perf] prepare_inputs: avoid O(seq_len) get_tokens(0) marshalling on the host (NVIDIA#16791) [None][perf] Optimize Blackwell fused MHC half-MMA kernel (NVIDIA#16799) [None][infra] Auto-update test durations from OpenSearch (last 7 days) [None][perf] Skip DeepGEMM clean_logits in DSA indexer prefill on custom top-k path (NVIDIA#16789) [None][feat] Support DeepSeek-V4 in layer_wise_benchmarks (NVIDIA#16774) [https://nvbugs/6465993][fix] use attention cache dtype for disaggregated transfer (NVIDIA#16505) [https://nvbugs/6463822][fix] Fix LTX2 CUDA graph test leak issue (NVIDIA#16775) [https://nvbugs/5948435][chore] Unwaive DeepSeekV3Lite test_nvfp4_4gpus CUTLASS ep4 fp8kv on RTXPro6000D (NVIDIA#16621) [TRTLLM-14417][fix] Exclude ADP/cuda-graph dummy requests from speculative-decode acceptance stats (NVIDIA#16571) ... Signed-off-by: Hannah Zhang <hannahz@nvidia.com>
Summary
enable_cuda_coreinNVFP4Linear.__init__and_trtllm_fp8_prequant_linear_core()gate the CUDA core scaled-mm fast path for small M dimensions (M <= 8). Both checks match SM89(8,9)and SM120(12,0)but not SM121(12,1), leaving the fast path dead on DGX Spark GB10.This adds
(12,1)to both checks so SM121 gets the same fast path as SM120.Note on device-0 hardcoding
Both
linear.py:2571(torch.device("cuda:0")) andquant.py:111(torch.cuda.get_device_capability(0)) query device 0 at init time rather than the device the tensors will actually live on. This is a pre-existing issue that affects SM89 and SM120 equally — fixing it requires moving the check to dispatch time, which is a larger refactor beyond this PR scope.Test plan
enable_cuda_core=Trueon DGX Spark GB10 (SM121) after fixFixes #15673