-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[https://nvbugs/6550749][fix] Size the DeepGEMM warmup bucket stride to the 16-token config quantum #17242
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
chenfeiz0326
wants to merge
4
commits into
NVIDIA:main
from
chenfeiz0326:fix/deep-gemm-warmup-bucket-stride-6550749
Closed
[https://nvbugs/6550749][fix] Size the DeepGEMM warmup bucket stride to the 16-token config quantum #17242
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
570c57e
[https://nvbugs/6550749][fix] Size DeepGEMM warmup buckets to the 16-…
chenfeiz0326 247dcaf
[https://nvbugs/6550749][fix] Add copyright header, return type and d…
chenfeiz0326 4be5ddd
[https://nvbugs/6550749][fix] Honour a declared tune_max_num_tokens i…
chenfeiz0326 7f90d2e
[https://nvbugs/6550749][fix] Keep the bucket clamps inside the x>=12…
chenfeiz0326 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will increase the tuning numbers a lot. Is it expected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, expected — but you're right that it's more than it needs to be, and I'd like to fix half of it.
Measured cost. 46 -> 264 buckets, and the autotune window (FMHA JIT warmup marker -> "Run warmup") goes 107.6s -> 110.3s, so +2.7s (+2.5%) at ~12.2ms per extra bucket. Compiles only go 33 -> 37 cubins: the distinct-config count is bounded by the
BLOCK_Mladder (~17/shape), not by bucket density, so the extra buckets are warm GEMM calls, not nvcc. They also cannot change steady-state kernel choice — selection is a function of M either way.Where the waste actually is. You're right that the grid is mostly redundant: 264 buckets collapse to 37 distinct configs (86% duplicates). But the stride isn't the culprit — a greedy minimum cover over the conservative
BLOCK_Msuperset needs 263 buckets, versus my 264. Stride 16 is essentially minimal for a guaranteed cover, because a band can be exactly 16 wide (measured: stride 32 leaves M=385 cold).The real waste is the pre-existing
max(x, 4096)floor. This workload ismaxnt:2048, so:maxnt:1024maxnt:2048maxnt:4096So ~48% of the buckets on this case tune M values the workload can never reach. My standalone coverage probe measured exactly that reachable-clamped stride-16 set — 136 buckets — and it came back with residual 0 on a stride-1 sweep, i.e. provably complete for both live shapes. Same coverage, half the buckets, ~1.6s cheaper.
Why I didn't just drop the floor here.
fp8SwapABGemmRunnerleavestune_max_num_tokens=None, soautotuner.py:1579-1582hands this function the current input size, not a maximum. Remove the floor and a small first call (say M=64) warms nothing above 120 — reintroducing the same class of hole. The floor is a workaround for the runner not declaring its max.The clean fix, if you're happy with it: plumb a real max through, exactly as the neighbouring runners already do —
Fp8BlockScalingGemmRunnerpinstune_max_num_tokens=4096and MoERunner 8192 (torch_custom_ops.py:86, 233, 1957, 2552).fp8_swap_ab_gemmgets atune_max_num_tokensarg,linear.py:1152passes the model's realmax_num_tokens, and the floor goes away. Then the count is 136 atmaxnt:2048and 264 only when the workload genuinely reaches 4096 — strictly fewer buckets than today in every case, and it removes the guessing.That's a slightly wider change than a warmup-stride bugfix, so I didn't fold it in unasked. Happy to do it in this PR, or land the stride fix (which is what removes the 2.2s mid-inference nvcc stall and the +55.9% end-to-end) and follow up with the plumbing — your call.
One note on the diff you're looking at: the second half of the change is also a bugfix, not just a stride change.
range(128, x, 16)is half-open, so the band containingmax_num_tokensitself was never warmed — the M every full batch runs at. That hole was 128 wide before this change and it's whyFp8BlockScalingGemmRunnerhad[3969, 4096]cold.