[GG] Revert unconditional MLA BMM contiguity workaround - #138
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughMLA attention removes backend BMM contiguity flag handling across initialization, decoding, weight processing, and V up-projection. Corresponding contiguity tests are deleted, while runtime weight-address preservation assertions remain. ChangesMLA BMM contiguity behavior
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reverts #136 as a whole before replacing it with a producer-side DCP buffer fix.
Why
The backend contiguity contract introduced per-layer input and output copies for every B12X MLA invocation, including DCP1. Release validation measured A16 DCP1/MTP0 decode falling from 87.14 to 82.52 tok/s (~5.3%), while 64K prefill remained near parity.
The Xid31 report itself is valid: a guarded VMM reproducer proves that the selected cuBLAS strided BMM reads to base +64 KiB for the affected shape. The safe and fast fix belongs at DCP output allocation: provide mapped tail padding from B12X A2A and NCCL fallback producers instead of copying every MLA BMM operand.
This PR intentionally contains only the exact revert. A separate clean PR with tail-padded DCP outputs and unit/E2E coverage follows from this reverted base.
Summary by CodeRabbit
Performance
Tests