Skip to content

[None][fix] Map SM107 to the sm_100f NVRTC target in the trtllm-gen FMHA kernel loader - #18758

Merged
farazkh80 merged 1 commit into
NVIDIA:mainfrom
farazkh80:rubin_feat/sm107_dsv4_fmha_riders
Sep 11, 2026
Merged

farazkh80 merged 1 commit into
NVIDIA:mainfrom
farazkh80:rubin_feat/sm107_dsv4_fmha_riders

Conversation

@farazkh80

@farazkh80 farazkh80 commented Sep 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

Follow-up to #18518, found while running DeepSeek-V4 end to end on a 4-GPU SM107 machine on top of main + #18518.

The trtllm-gen FMHA target is built without the Rubin feature define, so tg::CudaArch::Sm107a and the gated case 107 in intToCudaArch compile out and an SM107 device falls to default (assert in debug, silently Sm100a in release). Any FMHA config that takes the NVRTC path is then compiled for sm_100a, which does not load on a 10.7 device. Return the family arch (Sm100f) instead. Precompiled Sm107a cubins are unaffected (selection goes through mSM / isSMCompatible); cudaArchToInt already maps Sm100f → 100 and has no callers.

Scope

  • Path: cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h (+3).
  • Claim: SM107-only behavior change; no-op everywhere else — the new case 107 sits in the #else of the existing feature-define block and only matches smVersion == 107; the other cases are untouched.

Test coverage

Run on 4× SM107 (DeepSeek-V4-Flash, TP4/EP4, attention-DP, MoeConfig(backend="TRTLLM"), KvCacheConfig(dtype="fp8"), CUDA graphs), main + #18518 + this commit: quickstart passes with the trtllm-gen FMHA JIT warmup running on all ranks; GSM8K (1319, 5-shot) 95.38 vs the 95.11 (8× B200) / 95.38 (4× B300) references in references/gsm8k.yaml. Blackwell behavior unchanged by construction; existing FMHA tests cover the unchanged paths.

Note for SM107 users: with the current nvidia-cutlass-dsl pin (4.6.2) the DeepSeek-V4 CuTe DSL kernel selection still fails on SM107 (KeyError: 'sm_107a'); that resolves with the planned pin bump to a cutlass-dsl that knows sm_107a (4.8.0.dev0 does) and is intentionally not worked around here.

PR Checklist

  • PR title follows [None][fix] ...
  • Commit signed off (DCO)
  • No new dependencies, no version-pin changes
  • SM107-only behavior change; other architectures unaffected

🤖 Generated with Claude Code

Dev Engineer Review

  • Maps SM107 to Sm100f for NVRTC builds without TLLM_RUBIN_FEATURES.
  • Preserves Sm107a mapping for Rubin-feature builds and precompiled cubin selection.
  • Limits the change to SM107.
  • No public API or declaration changes.
  • Validation passed for quickstart and GSM8K on a 4-GPU SM107 setup.

Verdict: sufficient

QA Engineer Review

No test changes.

…iled out

The trtllm-gen FMHA target is built without the Rubin feature define, so
tg::CudaArch::Sm107a and the gated `case 107` compile out and an SM107 device
falls through to `default` (assert in debug, silently Sm100a in release).
Kernels that take the NVRTC path are then compiled for sm_100a, which does
not load on a 10.7 device. Return the family arch instead: the precompiled
Sm107a cubins are unaffected (selection goes through mSM and
isSMCompatible), and NVRTC emits sm_100f SASS that SM107 can run.

No-op for every other arch: the new case sits in the #else of the existing
feature-define block and only matches smVersion == 107.

Signed-off-by: Faraz Khoubsirat <58580514+farazkh80@users.noreply.github.com>
@farazkh80
farazkh80 force-pushed the rubin_feat/sm107_dsv4_fmha_riders branch from 37f75dd to e89e73f Compare September 5, 2026 13:56
@farazkh80 farazkh80 changed the title [None][fix] SM107 follow-ups to #18518: NVRTC arch fallback and DeepSeek-V4 CuTe DSL gate [None][fix] Map SM107 to the sm_100f NVRTC target in the trtllm-gen FMHA kernel loader Sep 5, 2026
@farazkh80
farazkh80 marked this pull request as ready for review September 6, 2026 21:58
@farazkh80
farazkh80 requested a review from a team as a code owner September 6, 2026 21:58
@farazkh80
farazkh80 requested review from BowenFu, PerkzZheng and yuxianq and removed request for PerkzZheng September 6, 2026 21:58
@coderabbitai

coderabbitai Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 32c6182b-2bc8-413e-a3ae-85fe5815c7a2

📥 Commits

Reviewing files that changed from the base of the PR and between 9964d34 and e89e73f.

📒 Files selected for processing (1)
  • cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The FMHA kernel architecture mapping now provides an SM107 mapping for builds without TLLM_RUBIN_FEATURES. Rubin builds retain their existing mapping.

Changes

SM107 architecture mapping

Layer / File(s) Summary
Add non-Rubin SM107 mapping
cpp/tensorrt_llm/kernels/trtllmGenKernels/fmha/fmhaKernels.h
Non-Rubin builds map SM107 to tg::CudaArch::Sm100f. Rubin builds continue to map SM107 to tg::CudaArch::Sm107a.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to e89e7

Non-Rubin SM107 systems now compile FMHA NVRTC kernels for the compatible sm_100f target while Rubin builds continue using sm_107a, avoiding incompatible kernel targeting without changing other architectures.

Suggested reviewers: chzblych

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the SM107 failure, the Sm100f fallback solution, scope, test coverage, and checklist status. It is complete and aligned with the repository template.
Title check ✅ Passed The title is concise, specific, and accurately identifies the SM107 mapping fix for the trtllm-gen FMHA NVRTC target.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@yuxianq
yuxianq requested a review from kris1025 September 7, 2026 02:02
@BowenFu

BowenFu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #71858 [ run ] triggered by Bot. Commit: e89e73f Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #71858 [ run ] completed with state SUCCESS. Commit: e89e73f
/LLM/main/L0_MergeRequest_PR pipeline #58934 completed with status: 'UNSTABLE'

CI Report

⚠️ Multi-GPU Label Required:
Multi-GPU tests require the ci: full pre-merge approved label on this PR. Ask a member of NVIDIA/trt-llm-ci-approvers to add the label, then re-trigger CI with the same bot command (no rebase needed).

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72776 [ run ] triggered by Bot. Commit: e89e73f Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72776 [ run ] completed with state SUCCESS. Commit: e89e73f
/LLM/main/L0_MergeRequest_PR pipeline #59761 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72803 [ run ] triggered by Bot. Commit: e89e73f Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72803 [ run ] completed with state FAILURE. Commit: e89e73f
/LLM/main/L0_MergeRequest_PR pipeline #59787 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

CI Agent Failure Analysis

Link to invocation

@farazkh80

Copy link
Copy Markdown
Collaborator Author

/bot run --disable-fail-fast

@farazkh80
farazkh80 enabled auto-merge (squash) September 11, 2026 14:21
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72950 [ run ] triggered by Bot. Commit: e89e73f Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #72950 [ run ] completed with state SUCCESS. Commit: e89e73f
/LLM/main/L0_MergeRequest_PR pipeline #59913 completed with status: 'SUCCESS'

CI Report

Link to invocation

@farazkh80
farazkh80 merged commit a85c26c into NVIDIA:main Sep 11, 2026
19 checks passed
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.

5 participants