[FlyDSL] Keep split-K preshuffle buffers out of the CUDA graph pool - #5393
Open
PerryZhang01 wants to merge 1 commit into
Open
PerryZhang01 wants to merge 1 commit into
PerryZhang01 wants to merge 1 commit into
Conversation
`_get_preshuffle_split_buffers` was keyed on `(device, stream)`. A CUDA graph
capture always runs on a fresh stream, so a capture was a guaranteed cache
miss and the workspace/semaphore were allocated *inside* the capture region.
Memory obtained there comes from that graph's private mempool, and the caching
allocator may hand the same block out again during a later capture into the
same pool -- vLLM captures every graph into one shared pool. The cached
buffers then alias another graph's tensors; every replay of that graph
overwrites the semaphore, the reduction's arrival count goes wrong, and it
reduces workspace slots the launch never wrote.
Key on device alone and refuse to allocate while a graph is capturing.
Dropping the stream is what makes preallocation possible at all: a capture
stream does not exist before the capture starts, so a stream-keyed cache can
never be warmed ahead of one. `preallocate_preshuffle_split_buffers()` is
exported for integrators that capture graphs.
Measured on gfx950, no vLLM involved -- three streams plus one real capture:
before: 3 distinct buffer sets, cache 0 hits / 3 misses,
is_current_stream_capturing() True at the allocation
after: 1 buffer set, cache 3 hits / 1 miss,
the capturing call is a hit; a cold cache in capture raises
End to end this was found as NaN draft logits in Kimi-K3 DSpark speculative
decoding on 8xMI355X: the draft's layer-0 gate_up_proj (N=3584, K=7168) took a
_ks4/_ks2 split-K kernel at small M, the reduction returned ~1e30..inf garbage,
SiLU overflowed to inf, and down_proj's per-token quantization turned the whole
row NaN. A byte histogram of the bad output shows a sign-symmetric 6e29..inf
spread with NaN and inf rows in the same step -- foreign data read as float32
partial sums, not a missing reduction term. With this change the same run is
clean: gsm8k 5-shot 0.9515, 1319/1319, zero faults, no NaN across 3580 steps,
peak |gate_up| 11.1 against 3.39e38 before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
PR title tags & labels: |
Collaborator
|
not a correct direction.. |
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.
_get_preshuffle_split_buffers(added in #5007) is keyed on(device, stream).A CUDA graph capture always runs on a fresh stream, so a capture is a
guaranteed cache miss and the split-K workspace/semaphore are allocated
inside the capture region. That is the defect.
A buffer allocated during capture and then kept alive by the
lru_cachetakes over a block that a transient tensor released earlier in that same
capture, or in an earlier capture into the same pool, was using — and the
kernels writing to that address are already recorded in the graph. The
allocator knows the transient was freed; it does not know a graph still writes
there. So every replay of that graph writes its own intermediate values
straight into aiter's live workspace and semaphore.
The semaphore is what turns this into a wrong answer.
torch.zerosran insidethe first capture, so its zeroing kernel is baked into that graph, which
therefore self-heals on every replay. Every later-captured GEMM graph is an
lru_cachehit — same buffers, no zeroing. When a dirty slot reaches one ofthose graphs,
splitk_reduce_epiloguenever seesarrival == split_k - 1, the reduction for that tile never fires, and theoutput tile is silently left holding whatever was in it.
Note the direction: this is not "a later graph steals the live block".
PyTorch's live accounting is correct and a later capture cannot take a block
the cache still holds (arm C below). The cached buffer is the one that
inherited someone else's address.
The fix keys on device alone and refuses to allocate while a graph is
capturing, with
preallocate_preshuffle_split_buffers(device)exported sointegrators can warm the cache during their own warmup.
Why dropping the stream is required, not incidental
A capture stream does not exist before the capture starts, so a stream-keyed
cache can never be warmed ahead of a capture — no amount of warmup makes
the first capturing call a hit. Keying on device is what makes preallocation
possible at all.
Why a capture-time allocation is unsafe — minimal standalone proof
Four arms, pure PyTorch, one GPU, 1 MiB, seconds. No aiter, no vLLM, no model.
Each arm allocates a long-lived buffer, fills it with a canary from eager, and
replays.
Tfreed, thentorch.emptyforWST.ptr == WS.ptrTrue6.0=T's resultWSallocated outside the captureWS; graph 2 captures afterWS's address FalseWSalone; graph 1's replay writes itTduring capture; graph 2 allocatesWSduring its captureT.ptr == WS.ptrTrue6.0; replay graph 2 ->15.0; both write itArm C is the one worth reading twice: the allocator is not at fault and the
"stolen live block" story does not hold. Arm A shows the second writer can be
the very same graph, with no other graph involved. Arm D shows that with a
shared
graph_pool— which is what vLLM uses — any previously captured graphqualifies.
This is exactly the situation a long-lived
lru_cachebuffer creates, and itis why the rule "do not let capture-time allocations outlive the capture"
exists. Arm B is the fix.
repro script (tested on torch 2.10.0+rocm7.2.4, gfx950; plain CUDA works too)
Expected output:
The same thing, in aiter, producing a wrong GEMM result
The arms above are pure PyTorch. This one drives the real
flydsl_preshuffle_gemm_a8— still no model and no vLLM, one GPU,M=1 N=2048 K=2048withsplit_k=4(a real_ks4row from a tuned table). All graphs arecaptured onto one side stream into one shared pool, as
vllm.distributed.parallel_state.graph_capture()does for every batch size.A decoy graph allocates a transient and frees it mid-capture. Then GEMM graph
g1is captured (lru MISS — allocates the buffers), then GEMM graphg2(lru HIT — same buffers, no zeroing kernel of its own).
g1captured firstg2captured laterg1g2Deterministic over 3 runs. The arithmetic closes exactly: this shape uses 32
semaphore slots (
N/tile_n = 2048/64), of which exactly one is dirty, andone tile is
tile_n = 64columns — so 64 wrong outputs.The collision is not engineered. The dirty slot holds
1270874112, which is0x4BC00000, which is float3225165824.0, which is6.0 * 4194304— thedecoy graph's
T.sum()scalar. A 4-byte reduction result landed onaiter's semaphore slot 0.
Note what the workspace alone does not do: it is clobbered in full
(4194304/4194304 elements) and
g1still returns the right answer, because alaunch writes every plane it reduces. The wrong answer comes from the
semaphore. Depending on the garbage value the epilogue either never fires (what
is measured above — a stale output tile) or fires early, in which case the
reduction reads workspace planes the launch has not written yet; the latter
would produce the out-of-range partials seen on K3 below, but this repro
demonstrates the former.
repro script
Run it against an aiter build that still has the stream-keyed cache; the
"fixed" arm monkeypatches this PR's behaviour (device-keyed, preallocated
before any capture) so both arms run in one process.
Expected output:
Measurement — gfx950, standalone, no vLLM
Three streams plus one real
torch.cuda.CUDAGraph()capture:hits=0, misses=3is_current_stream_capturing()True at the allocationhits=3, misses=1How this was found — Kimi-K3 DSpark, 8xMI355X
NaN draft logits in speculative decoding. The draft's layer-0
gate_up_proj(N=3584, K=7168) takes a
_ks4/_ks2split-K kernel at small M(M = num_seqs x 7, so the tail of a dataset run drops into the split-K
buckets while mid-run concurrency 64 does not — which is why it only ever
failed near the end). The reduction returned ~1e30..inf garbage, SiLU
overflowed to inf, and
down_proj's per-token quantization turned the wholerow NaN.
A byte histogram of the corrupt GEMM output shows a sign-symmetric
6e29..inf spread, with NaN and inf rows present in the same step
(
7f:13535vsff:13448in the high byte). That is foreign data being readas float32 partial sums — not a missing reduction term, which would be
bounded and one-sided.
Single-variable runs on the same configuration, changing one thing each time:
streamdropped from the cache keyRun C full acceptance, 5-shot gsm8k, 1319 questions, TP8 + DCP8, 7 speculative
tokens:
flexible-extract 0.9515 +-0.0059,strict-match 0.9507,1319/1319 with zero faults,
nan_rows=0across 3580 steps / 271768 rows.The score is mid-range for this configuration (repeats span 0.9484..0.9598) —
the fix removes the fault without moving accuracy, because drafts are verified
by the target, so a corrupt draft costs acceptance rate rather than answers.
Tradeoffs — both deliberate, both worth stating
Concurrent split-K launches on different streams now share one buffer
set. The original code already assumed launches are ordered and that the
reduction hands the semaphore back zeroed; this extends that same
assumption across streams instead of within one. (That zeroing protocol is
not what failed here — it only guards against the split-K path's own stale
state, and the 16 MiB workspace, not the 1 KiB semaphore, is what a graph
replay overwrites.) Given the alternative is a
cache that is structurally un-warmable before a capture, this is the right
side of the trade — but it is a real widening and should be reviewed as one.
A first call landing inside a capture now raises instead of silently
corrupting. For vLLM this is safe (the first call happens during warmup,
is_current_stream_capturing()is False). For an integrator whose veryfirst split-K call is inside a capture, behaviour changes from "produces
garbage" to "fails at startup with a message naming the fix". I think loud
is correct here, but it is a behaviour change, not a pure bugfix.
Tests
op_tests/test_flydsl_splitk_buffers.py(new) pins four properties: buffersare shared across streams; allocation during capture is refused with an
actionable message; preallocation lets a capture reuse them; and a real
CUDAGraphcapture allocates nothing (data_ptr()unchanged). All four failagainst the pre-fix module and pass after.
black --checkandruff==0.16.0 checkare clean on both files.cc @XiaobingSuper (author of #5007)
🤖 Generated with Claude Code