fix: align is_sm120f_supported with SM12x family semantics - #3175
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Code Review
This pull request updates the is_sm120f_supported function in flashinfer/utils.py to simplify the compute capability check by only verifying the major version (12) and removing the specific minor version check. I have no feedback to provide.
|
/bot run |
## 📌 Description After #3113 extended the b12x decorator to SM121, calling `mm_fp4(..., backend="b12x")` on sm_121 still trips the dispatch-side check in `dense_blockscaled_gemm_sm120_b12x.py:1591`: ``` ValueError: dense_gemm launch only supports sm_120, got sm_121 ``` SM120 and SM121 share the same 12.x spec (MMA atoms, SMEM), so the same kernel applies. Allow sm_121 through the check. The error message is updated to match. Other call sites in this file (`sm_version="sm_120"` at line 1875, `get_smem_capacity_in_bytes("sm_120")` at lines 122/1461) are left alone — they work for both arches. ## 🔍 Related Issues Refs #3170 (Action Item 4). Follows up on #3113. ## 🚀 Pull Request Checklist ### ✅ Pre-commit Checks - [x] I have installed `pre-commit` by running `pip install pre-commit` (or used your preferred method). - [x] I have installed the hooks with `pre-commit install`. - [x] I have run the hooks manually with `pre-commit run --all-files` and fixed any reported issues. ## 🧪 Tests - [ ] Tests have been added or updated as needed. - [ ] All tests are passing (`unittest`, etc.). No new tests; no SM121 hardware locally. Existing sm_120 path unchanged. ## Reviewer Notes Same audit cleanup batch as #3173 / #3174 / #3175. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Improvements** * Expanded GPU architecture support to include additional SM12x devices (sm_120 and sm_121). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
📌 Description
is_sm120f_supportedrequiresminor == 0, so it returns False on sm_121 even though the120fgencode target covers both sm_120 and sm_121. Drop theminor == 0check.No callers in
flashinfer/ortests/. OnlyCLAUDE.mdmentions it. Could delete the function instead if that's cleaner.🔍 Related Issues
Refs #3170 (Action Item 5 / A1).
🚀 Pull Request Checklist
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(orused your preferred method).
pre-commit install.pre-commit run --all-filesandfixed any reported issues.
Ran
pre-commit run --files flashinfer/utils.py; passed.🧪 Tests
unittest, etc.).No new tests; the function is currently uncalled.
Reviewer Notes
Summary by CodeRabbit