Update moe gemm - #3239
Update moe gemm#3239
Conversation
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
|
/bot run |
|
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)
📝 WalkthroughWalkthroughAdds SM detection and architecture guards to TRTLLM batched GEMM config selection when patchF2fp is enabled; updates low-latency GEMM kernel name constants; and updates prebuilt TRTLLM artifact paths and checksums. ChangesTRTLLM Architecture Gating & Artifacts
Low-Latency Kernel Configuration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
![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 updates GEMM configuration filtering for SM 100 and 103, cleans up kernel names, and updates artifact hashes. Feedback indicates that the filtering logic for mPatchF2fp is incorrectly placed after the configuration is already accepted, rendering it ineffective. Additionally, the use of std::cerr for error reporting is noted as redundant and should be integrated into the existing FLASHINFER_CHECK macro to better align with library logging practices.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@csrc/trtllm_batched_gemm_runner.cu`:
- Around line 124-133: The push of index i into mPassingConfigIndices happens
before the SM/patchF2fp checks so incompatible configs slip through; move or
conditionally perform mPassingConfigIndices.push_back(i) after the patchF2fp/SM
filtering (the blocks that check options.mPatchF2fp, sm_version, and config.mSm
against tg::CudaArch::Sm103a/Sm100a/Sm100f) and also ensure the
transposeMmaOutput check (mOptions.transposeMmaOutput && options.mEpilogueTileM
== mOptions.epilogueTileM) is evaluated only after those SM guards pass, so that
only configs that survive the SM filters get pushed into mPassingConfigIndices.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b015f9b5-b3e5-4c65-86bd-821c96ec416b
📒 Files selected for processing (3)
csrc/trtllm_batched_gemm_runner.cucsrc/trtllm_low_latency_gemm_runner.cuflashinfer/artifacts.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@csrc/trtllm_batched_gemm_runner.cu`:
- Around line 94-95: The code snapshots SM via getSMVersion() at construction
which can differ from the device used in run(), causing incorrect entries in
mPassingConfigIndices for patchF2fp; either (A) bind the runner to a single
device by storing the intended device ID in the constructor and assert that the
device argument passed to run() matches that stored ID before using sm_version
(so the original SM-based filter remains safe), or (B) remove the SM-based
filtering from construction and instead perform the SM filtering of patchF2fp
when computing valid configs in getValidConfigIndices() or inside run() where
the explicit device argument is available (call getSMVersion() for that device
there) and then populate mPassingConfigIndices based on that device-specific SM
version. Ensure you update references to getSMVersion(), mPassingConfigIndices,
run(), getValidConfigIndices(), and patchF2fp accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee9b459e-38e9-4358-94dc-36b3c22d74d4
📒 Files selected for processing (1)
csrc/trtllm_batched_gemm_runner.cu
| auto sm_version = getSMVersion(); | ||
|
|
There was a problem hiding this comment.
Do not make SM-specific config admission depend on the current device at construction time.
Line 94 snapshots getSMVersion() from whichever CUDA device is current during construction, but the runner later executes against an explicit device argument in run(). That means a runner created on one GPU and reused on another can permanently admit the wrong patchF2fp configs into mPassingConfigIndices, which later drives getValidConfigIndices(). Please either bind the runner to a single device and assert that on use, or move this SM filtering to a path that already knows the target device.
Also applies to: 124-130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@csrc/trtllm_batched_gemm_runner.cu` around lines 94 - 95, The code snapshots
SM via getSMVersion() at construction which can differ from the device used in
run(), causing incorrect entries in mPassingConfigIndices for patchF2fp; either
(A) bind the runner to a single device by storing the intended device ID in the
constructor and assert that the device argument passed to run() matches that
stored ID before using sm_version (so the original SM-based filter remains
safe), or (B) remove the SM-based filtering from construction and instead
perform the SM filtering of patchF2fp when computing valid configs in
getValidConfigIndices() or inside run() where the explicit device argument is
available (call getSMVersion() for that device there) and then populate
mPassingConfigIndices based on that device-specific SM version. Ensure you
update references to getSMVersion(), mPassingConfigIndices, run(),
getValidConfigIndices(), and patchF2fp accordingly.
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
|
/bot run |
Signed-off-by: Siyuan Fu <siyuanf@nvidia.com>
|
/bot run |
|
/bot run |
3 participants
📌 Description
Update the trtllm-gen cubin to improve mxfp4 moe, DeepSeek-V3 MoE, and mxfp8 gemm performance.
🔍 Related Issues
🚀 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.).Reviewer Notes
Summary by CodeRabbit
New Features
Updates