fix(moe): pad BF16 TRTLLM-Gen intermediates to 128 KiB - #4319
Conversation
Signed-off-by: Po-Han Huang <pohanh@nvidia.com>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughBF16 MoE GEMM1 and GEMM2 output buffers now use separate minimum token capacities derived from output dimensions and BF16 element size. ChangesBF16 MoE capacity allocation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
/bot run |
|
[FAILED] Pipeline #60808548 — 11/18 executed test jobs passed Compared with nightly #60712014. Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 4/6 passed
Failure detailsNew relative to nightly (attribution uncertain)
Pre-existing failures
Timeouts, infrastructure, or incomplete jobs
|
|
This will fix #3427 |
|
/bot run |
|
[FAILED] Pipeline #61135895 — 8/18 executed test jobs passed Compared with nightly #60831563. Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 4/6 passed
Failure detailsNew relative to nightly (attribution uncertain)
Timeouts, infrastructure, or incomplete jobs
|
|
/bot run |
|
/bot run tests/moe |
|
[FAILED] Pipeline #61461786 — 14/18 executed test jobs passed Compared with nightly #61182354. Unit Tests
✅ Pass · 🟡 Old failure · ❌ New failure · ⏱ Test timeout · Multi-GPU and Multi-Node Tests — 4/6 passed
No individual test or infrastructure failures could be extracted. |
|
Hi, is there anything blocking the merge of this PR? It occurs during GLM 5.2 draft MoE, so ideally I'm wondering if it can be merged soon |
|
/bot run tests/moe |
|
[SUCCESS] Pipeline #61889462: 18/18 executed test jobs passed |
|
@nvpohanh dumb question why not consider the cutlass fix for tma descriptor? since it seems like it is the same issue? or am I wrong? |
|
TBH I think that fix is a little ugly 😹 And padding workspace to 128KiB should be almost harmless |
Hatch SessionThis PR was worked on in a Hatch coding session.
|
[by Codex]
Summary
Routing::maybeGetMinTokenCount, matching the existing FP8/MXFP launchersWhy
On Blackwell, the TRTLLM-Gen BMM2 kernel can use TMA
BASE_128KBaddress generation based on the logical tensor-map shape. With a small eager decode batch, the BF16 launcher currently allocates only the logical 16/32-KiB intermediate. That leaves less than 128 KiB mapped after the activation base and can produce an illegal memory access inUTMALDG.4D.The quantized launchers already call
maybeGetMinTokenCountfor both intermediates. Applying the same allocation policy to the BF16 launcher gives the TMA transaction the required backing without changing the logical token count passed to routing or GEMM.Validation
pre-commit run --all-filesResult: six consecutive valid passes on B200x8. Every run exited normally, completed all four steps, passed the diagnostic evidence gate, and had zero IMA/sanitizer/segfault matches.
Summary by CodeRabbit