metal: prune top-k argsort merge rounds to top_k - #959
Open
emilianbold wants to merge 2 commits into
Open
Conversation
kernel_argsort_merge_f32_i32 carried work_width elements per row through every intermediate round, so the full row was re-merged until the final merge clamped to top_k, and the thread split divided the number of elements read rather than written. Reads now walk the input runs at stride 2*len bounded by args.total (the valid elements per row); writes pack at stride new_len = MIN(2*len, keep_k); the work split divides out = MIN(total, new_len), so no thread exits early when pruning binds. Odd run counts leave the last run without a partner, which falls out of len1 == 0 and copies it through. The host tracks len/total/nruns per round and drives the loop off the run count instead of len < work_width. total_next is O(1): full pairs write new_len, the last partial pair writes MIN(r, new_len), with q clamped to the dispatched pair count. The final round always has new_len == top_k, since nruns == 2 bounds total by 2*len and total never drops below top_k. For n_comp=8192, top_k=512 the element writes per row across all merge rounds drop from 8704 to 3584.
ds4_gpu_indexer_topk_tensor against a host qsort reference, over shapes the CUDA-only run_topk2048 in test_gpu_xdev.c cannot reach: odd run counts with a leftover run passed through, top_k small enough to prune from the first merge, non-power-of-two top_k, the degenerate top_k=1 reduction, the one-pass path where the merge kernel never runs, and n_tokens=1 (decode) as well as n_tokens=2. Scores are a per-row permutation of 0..n_comp-1, so every score is unique and float-exact and the comparison does not depend on tie-break order, which the merge resolves left-run-first. print_path mirrors the host shape math and prints nth/npr/work_width and the round count per case, so each case's path is confirmed at run time rather than inferred from the comment. Run with `make test-argsort-metal`.
emilianbold
force-pushed
the
metal-topk-prune
branch
from
September 4, 2026 07:44
6353e91 to
ea16db6
Compare
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.
Summary
kernel_argsort_merge_f32_i32carriedwork_widthelements per row through every intermediate merge round, so the full row was re-merged until the final merge clamped totop_k. It also divided thread work by the number of elements read rather than written, so once a round truncates, the high threads exit immediately and whole SIMD groups idle.This PR prunes each round to
top_kand splits the work over the outputs that actually exist. Same result, less work, better occupancy.The work was started with DeepSeek Flash on ds4 then refined with Claude.
The change
metal/argsort.metal:2*len, bounded by a newargs.total(the valid elements per row) instead ofargs.ne0(the buffer row stride).start_write = im * new_lenwherenew_len = MIN(2*len, keep_k), so a round keeps at mosttop_kper pair.out = MIN(total, new_len), and one guard (k0 >= out) replaces the two old early returns.total,keep_k.ds4_metal.m(ds4_gpu_indexer_topk_tensor):len/total/nrunsper round and drives the loop off the run count rather thanlen < work_width.total_nextis O(1): full pairs writenew_len, the last partial pair writesMIN(r, new_len), withqclamped to the dispatched pair count.Odd run counts need no special case: the last run has no partner, which falls out of
len1 == 0, and the merge copies it through.Why it is correct
Runs stay packed at stride
lenwith every run full but the last, so:total >= top_kis preserved every round;nruns == 2, hencetotal <= 2*len, hence2*len >= top_k, hencenew_len == top_kand exactlytop_kelements are written to the output tensor;total_next <= total <= work_width, so they stay in the row.Truncating a pair to
new_lenis exact rather than approximate: a dropped element had>= top_kelements above it inside the pair, hence>= top_kin the whole row, so it was not in the top-k.Performance
Merge-element writes per row, across all rounds, at
nth = 1024:Measured merge-GPU time per call, median over 7 runs on an idle GPU:
Two caveats on the measurement. It was taken with the merge rounds isolated in their own command buffer, which is not how they are encoded in production, and the harness that does that is not part of this PR.
End to end this is modest in an ordinary context: the saving is about 0.10 ms/token at Flash 32k summed over 43 layers, roughly 0.3% of a ~31 ms token. It grows with context though.
Testing
make test-argsort-metalrunstests/test_argsort_metal.c