Prevent overflow in sorted Top-K index offsets - #2
Open
morluto wants to merge 1 commit into
Open
Conversation
fire-emblem
pushed a commit
to CompilerFans/DeepSelect
that referenced
this pull request
Sep 12, 2026
…'s levers
The handover's §9 ranked four levers for the C500 row kernel, and the first two
rest on an inference rather than a measurement: "the cell is bound on per-CTA
memory-level parallelism" is a plausible reading of 28%-of-wall bandwidth, not
something anyone had checked. This measures it, and the measurement retires two
of the four.
Method: incremental ablation, because the kernel has no FLOPs to attribute and
`clock64()` probes perturb register allocation. Each variant deletes exactly
one thing and leaves the rest byte-identical, so the shared cost (launch, the
row-walk skeleton, the barriers) cancels in the difference. All variants lived
in an isolated copy at /tmp/dsprobe; the repository itself was never modified and
its tree stayed clean throughout.
Cell b4096-v16384-k512 (bf16, k=512), one process, L2 flushed, kernel time:
full kernel 619.6 us
pass 2 emit atomic -> store 583.9 emit serialization = 35.7
pass 1 only 253.3
pass 1, no smem atomics 150.8 pass 1's atomics = 102.5
adjacent-pair merge (P4) 657.6 SLOWER than the full kernel
DRAM floor, 1 trip 116.7 (x.sum() on the same 128 MiB)
DRAM floor, 2 trips 233.4
Three findings:
* pass 1's eight shared atomics per uint4 cost 102.5 us, 16.5% of the kernel.
P1 and P2 differ by that one statement, so the delta is clean.
* Reducing the atomics' *conflicts* is not a win: P4 merges adjacent element
pairs into one atomic (4x fewer) and comes out 38 us SLOWER. So the 102.5 us
is per-atomic issue cost, not bus contention -- which means the ballot /
match-any group-merge the handover proposed as lever deepseek-ai#2 would have been P4's
stronger version of the same mistake. It is retired before being written,
which is the whole value of measuring first: that code would have touched the
emit path, the most delicate part of the kernel.
* pass 1's walk is already at 80% of the achievable single-trip rate (150.8
against a 116.7 us floor), so lever deepseek-ai#1 has only ~34 us of room *in pass 1*.
It is still worth doing, but its target is pass 2's walk, not pass 1's.
P5 (both walks, no emit) reads 379.6 but is documented as untrustworthy in the
record: its counting loop carries a dependency the real kernel does not have, so
it over-measures the second walk, and it lets the compiler reschedule the walks
once the tail work is gone. It is reported as a bound, not used in any delta.
The record also settles the MACA primitive facts the next person would otherwise
re-derive from the headers: `__ballot_sync` is native (one `sicmp`,
`__clang_maca_device_functions.h:2202`) and 64-bit-only, `__match_any_sync` is a
32-iteration per-bit software loop (:1648), `__activemask()` is one read of the
wave mask, `__popc` does not exist (use `__popcll`), and the wave is 64 lanes --
so a 32-bit ballot mask silently drops half the wave and under-counts the
histogram without raising anything.
The handover's §9 is rewritten against these numbers (pass-2 tail attribution
first, then pass 1's register histogram, then MLP; the group-merge lever moved to
the rejected list), and both records cross-reference each other. A new document,
docs/C500-radix-profile.zh.md, carries the method, the full table, the P5 caveat,
the binary md5s, and the reproduction recipe.
No source file is modified -- this commit is documentation only -- so both gates
stand at their recorded values and xcore1600 is byte-identical.
Co-Authored-By: Claude <noreply@anthropic.com>
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
1 participant
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.
Fixes #1.
Sorted Top-K indices can overflow before conversion to int64: selecting index 1 with offset
2147483647evaluates a signed int32 addition instead of producing2147483648. Short rows also subtract the offset from the padding sentinel, which overflows for the default fill and offset-1.Widen selected indices before adding the offset. Initialize raw indices for the short-row sort and identify padding by indices outside the row's valid range. Write
idx_oob_fill_valuedirectly for those entries, removing sentinel offset arithmetic.Added CUDA tests for selection, exact-size, partial and empty rows, extreme offsets and fill values, and representable int32 outputs.
Validation: the sorted FP32,
MAX_TOPK=512translation units compile for SM100a with both int32 and int64 indices using NVCC 13.3. Python syntax compilation andgit diff --checkpass. Host copies of the original expressions reproduce UBSan failures; a candidate arithmetic model passes 45 boundary combinations under UBSan. These host checks and compilation checks are not kernel execution results.