[https://nvbugs/6566891][fix] Use FlashInfer FA2 for Gemma4 on SM120 and SM121 - #17557
Conversation
Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
WalkthroughGemma4 now selects ChangesGemma4 backend dispatch
Estimated code review effort: 2 (Simple) | ~10 minutes Mergeability Score: 🔵 Low · up to The change narrowly routes Gemma4 attention through FA2 on SM120/SM121 while preserving existing behavior elsewhere. It is mergeable with owner awareness that multimodal image-prefill using custom masks on the affected architectures still needs explicit correctness validation; no concrete failure is currently demonstrated. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/bot run --disable-fail-fast |
|
PR_Github #65582 [ run ] triggered by Bot. Commit: |
brnguyen2
left a comment
There was a problem hiding this comment.
Approving — the comments below are optional touch-ups, not blockers.
The fix lands in the right place and the mocked-get_sm_version tests are a reasonable way to pin behavior without SM12x hardware. Two things worth resolving before merge:
- The reported hardware is SM121, and validation was on SM120. The two arches share the trtllm-gen gap, so the fix should carry over, but a Spark confirmation before merge would close the loop.
test_modeling_gemma4.pyonly appears inl0_b200.yml, so CI exercises the trtllm-gen branch exclusively; the fa2 branch is covered only by attribute assertions with a mocked SM version. Nothing here actually runs an fa2 Gemma4 kernel in CI. Worth saying explicitly in the PR description which fa2 paths were hand-validated (text generation) and which were not (multimodal prefill), since the entry point in the linked bug is the VLM one.
|
PR_Github #65582 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #65731 [ run ] triggered by Bot. Commit: |
|
PR_Github #65731 [ run ] completed with state |
Gate TRTLLM-Gen on datacenter Blackwell capability and use FA2 elsewhere. Align Gemma4 backend selection and CUDA Graph coverage with the same capability check. Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
|
/bot run --disable-fail-fast |
|
PR_Github #65883 [ run ] triggered by Bot. Commit: |
|
PR_Github #65883 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #65939 [ run ] triggered by Bot. Commit: |
|
PR_Github #65939 [ run ] completed with state |
…and SM121 (NVIDIA#17557) Signed-off-by: Fanrong Li <23290157+lfr-0531@users.noreply.github.com>
Dev Engineer Review
trtllm-genonly on SM100f.pre-commit, SM120 H256/H512 checks, and a short NVFP4 generation with FP8 KV cache.Verdict: sufficient.
QA Engineer Review
Modified test functions:
test_all_layers_use_trtllm_genwas renamed totest_all_layers_use_trtllm_gen_on_sm100f.test_non_sm100f_layers_use_fa2was added.These tests are not listed in
tests/integration/test_lists/because no test-list files changed. The unit tests provide direct coverage for the updated backend-selection behavior.Verdict: sufficient.
Description
Gemma4 currently forces every
FlashInferAttentionlayer to use the TRTLLM-Gen inner backend. TRTLLM-Gen does not provide the required attention kernels for SM120/SM121, so this path reaches an unsupported-architecture assertion on DGX Spark (SM121).This change uses FlashInfer FA2 for Gemma4 on SM120 and SM121 while preserving the existing TRTLLM-Gen path on all other architectures. It covers both the H256 sliding-window layers and H512 full-attention layers used by Gemma4. The change is limited to backend selection and does not introduce public API changes or new dependencies.
Related issue: https://nvbugspro.nvidia.com/bug/6566891
Related PR: #16214
Test Coverage
CMAKE_CUDA_ARCHITECTURES=120-real.python3 -m pytest -q tests/unittest/_torch/modeling/test_modeling_gemma4.py::TestGemma4ModelDefaults: 7 passed, 2 subtests passed.nvidia-Gemma-4-26B-A4B-NVFP4configuration and verified that H256 sliding-window and H512 full-attention layers both selected FA2 on SM120.max_seq_len=max_num_tokens=6272. FlashInfer JIT compiled an SM120f FA2 kernel, and the run completed withE2E_OK.pre-commit run --all-filessuccessfully.PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.