Conversation
Signed-off-by: Anurag Mukkara <[email protected]>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughThe FMHA kernel enumerator adds an SM120 tiled configuration for head size 256. The runner excludes SM120/SM121 from the E4M3 non-tiled-kernel branch. Both files update their copyright year ranges through 2026. ChangesSM120 FMHA kernel selection
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Feature Suggested reviewers: Merge Risk: ⚪ Minimal · up to No identified kernel-selection issue remains; this change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Description
Use tiled fmha_v2 kernel for FP8 (E4M3) context attention with head_dim 256 on SM120/SM121. Targets nvidia/Qwen3.6-35B-A3B-NVFP4 which uses FP8 KV cache.
cpp/kernels/fmha_v2/setup.py: add a tiled64x128QMMA kernel for head_dim 256, alongside the existing non-tiled64x32kernel.fmhaRunner.cpp: restrict the forced non-tiled FP8 selection to SM89, so SM120/SM121 use the existing head-size heuristic, which selects tiled kernels for head_dim >= 256.Performance
fmha_v2 harness on RTX PRO 6K BSE, FP8 Q/K/V, head_dim 256, tiled vs non-tiled kernel.
The tiled kernel is faster on all 35 shapes tested: 2.03x - 3.52x, geomean 3.01x.
Test Coverage
The tiled kernel passes the fmha_v2 harness reference check on SM120 (
-epsilon 0.2) for paged KV and packed QKV; causal, padding, sliding-window and custom masks; ALiBi; GQA; chunked prefill; and FP8 and BF16 output.e2e accuracy verified by
tests/integration/defs/accuracy/test_llm_api_pytorch.py::TestQwen3_6_35B_A3B::test_nvfp4_w4a16Dev Engineer Review
cpp/kernels/fmha_v2/setup.pyremoves unnecessary f-string prefixes from two constant strings. This does not change generated output or kernel selection. The supplied diff does not support the stated tiled-kernel orfmhaRunner.cppchanges.QA Engineer Review
No test changes.
Per-File QA Perspective
cpp/kernels/fmha_v2/setup.py: The generated#endiftext remains unchanged. No observable behavior change requires QA verification.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.