fix(gemm): remove SM120 MXFP8 tile configs that exceed the SM12x shared-memory limit - #4013
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughSM120/SM121 MXFP8 GEMM now retains three CTA tile shapes, propagates workspace dispatch failures, uses automatic shared-memory carveout, reports shared-memory limits during initialization failures, and tests every advertised tactic for BF16 and FP16 outputs. ChangesSM120 MXFP8 GEMM
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The change removes unusable SM120 tile configurations, improves shared-memory error reporting, and updates coverage for all advertised tactics; no actionable merge-blocking risk remains beyond normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the 256x128 and 128x256 tile configurations for SM120 MXFP8 GEMM as they exceed the shared memory limits of SM12x devices. It also simplifies stage count logic, enhances error reporting with detailed shared memory usage information, and updates the test suite to dynamically verify all available tactics. Regarding the feedback, removing the try-catch block when querying workspace sizes could lead to unhandled runtime errors (e.g., when placeholder kernels are used), so it is recommended to retain it for robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…imit The 256x128 and 128x256 CTA tiles need 111616 bytes of dynamic shared memory, above the 101376-byte per-block limit every SM12x device reports (verified on RTX 5080, RTX PRO 6000 Blackwell and GB10), so their kernels can never initialize on any GPU this SM120-only module runs on. - remove the two tile configs, their dispatch cases and kernel instantiations (tactic count drops from 10 to 6, four fewer kernels to compile) - on initialize() failure, report the kernel's shared-memory need vs the device limit when that is the cause - drop the now-dead StageCount<2> branch and the workspace-size try/catch that only existed for the oversized configs - test every advertised tactic (bf16 and fp16 outputs) against a reference instead of asserting a hardcoded tactic count AI-assisted (Claude Code). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
20b2df0 to
cd3fa80
Compare
|
@flashinfer-bot run |
|
/bot run tests/gemm |
|
[FAILED] Pipeline #63744341 — 15/16 executed test jobs passed Compared with nightly #63648836. Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 6/6 passed
Failure detailsTimeouts, infrastructure, or incomplete jobs
|
|
/bot run tests/gemm |
|
@flashinfer-bot run |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
[FAILED] Pipeline #65907369 — 15/16 executed test jobs passed Compared with nightly #65814627 (different CI configuration). Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 6/6 passed
Failure detailsCould not compare
|
3 participants
📌 Description
The issue:
[MXFP8 SM120 gemm Runner] Failed to initialize cutlass MXFP8 gemm on sm120. Error: Error Internal.The fixes:
🔍 Related Issues
Addresses the MXFP8 item of #3170 (kept open; other items remain). Supersedes #3568; thanks @tgmerritt for the original diagnosis on GB10. Measuring the actual shared-memory requests showed the configs are unusable on all SM12x devices, so this PR removes them at the source instead of filtering them out at runtime.
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Verified on GB10: the full
tests/gemm/test_mm_mxfp8_sm120.pysuite passes and the module reports 6 tactics. The surviving tactics also pass the numeric suite on RTX 5080 and RTX PRO 6000 boards.Reviewer Notes
If a future SM12x part ships with more shared memory and the large tiles become viable, the place to bring them back is
getConfigs(), ideally with a static check of each tile's shared-memory need against the device limit.🤖 Generated with Claude Code
Summary by CodeRabbit
Performance
Bug Fixes
Tests