Repository navigation
Keep self-reading complete copies in program order - #158
Merged
Merged
Conversation
Storage lowering turns a complete, contiguous copy into a persistent region into Inductor's post-graph writeback when no later write invalidates its source. A source that reads the destination's own region was also eligible. Inductor then inlines the source gather into the in-place store, so one kernel loads rows that other program instances are overwriting. Two row expansions write into the storage they read: the vocabulary projection expands gathered logits into sample slots when the gathered rows are a prefix view of its output, and the DeepSeek V4.1 decoder expands head hidden rows into scratch.hidden. With speculative decoding, DeepSeek V4.1 Flash returns wrong tokens, including tokens from other requests, when two or more requests are verified in one round, and draft acceptance falls at four or more concurrent requests. A complete copy becomes a writeback only when its source reads no region the copy writes. Other copies stay ordered storage calls, so the gather completes before the destination is written. Safe complete copies keep fusing into the writeback. The package format is unchanged; existing packages must be re-exported to compile with this rule. Validation: - GB10 (SM121): without this change the CUDA row-expansion test mismatches 58% of elements after one replay; with it the test passes in three runs. tests/test_compilation.py, tests/preparation/test_forward_compile_options.py and tests/preparation/test_invocation_views.py: 51 passed; the bf16_add_out native-replay test fails with and without this change because the installed Heliopause lacks the qualification feature. - CPU: the copy-lowering test requires an overlapping gather to stay an ordered copy; it fails without this change. - DeepSeek V4.1 Flash, TP4 on four DGX Spark hosts, 8192-token prefill chunk, speculation enabled. The exported package used this rule written as a set intersection; its tree also carried an uncommitted preparation allocator change needed for that chunk size. Temperature-0 record lookups and counting prompts were correct serially (32/32), at concurrency 8 with shared and with distinct prompts (32/32 each), at concurrency 4 (24/24) and at concurrency 2 (12/12); without this change the same checks gave 10-18/32, 18/24 and 11/12. Compiled forwards keep 37.4 complete copies per forward as writebacks (38.8 without this change) and contain no in-place gather-store kernels on the logits or head hidden buffers. Draft acceptance per offered draft is 0.73-0.77 at concurrency 4 and 0.55 at concurrency 8, against 0.41-0.50 and 0.33-0.44. In two llm_inference_bench passes, concurrency-4 decode is 144-151 tok/s against 115-128 tok/s; other cells stay within run-to-run variation. Limitation: the first launch of that package after rank packages were distributed exited after two minutes on a RoCE collective timeout while rank 0 had about 8 GiB in swap. Relaunches served 244 checked requests and two benchmark passes without failure; the cause was not traced. Design-Signed-Off-By: flare-02; design-review=1c01
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.
Problem
Storage lowering (
_lower_storageinignition/preparation/compilation.py) turns a complete, contiguous copy into a persistent region into Inductor's post-graph writeback when no later write invalidates the copy's source. That rule came from 317c1ec and was amended by cbb7d9d. It does not consider a source that reads the destination's own region.Two row expansions from 022c33b have that shape:
ignition/components/vocabulary.py: when the vocabulary shards cover the output width exactly,gathered = output[:rows], and__call__runsoutput.copy_(gathered[row_indices, :V]).ignition/recipes/deepseek41/decoder.py:head_hidden = scratch.hidden[:head_rows], thens.hidden.copy_(head_hidden[samples]).Inductor inlines each gather into the in-place writeback. In a compiled 12-row DeepSeek V4.1 speculative forward (vocabulary 129280), one Triton kernel loads from and stores to the same logits buffer:
Program instances overwrite rows that other instances still read, and token selection uses those rows. The head hidden buffer has the same pattern.
With speculative decoding, DeepSeek V4.1 Flash (TP4, four DGX Spark hosts) returns wrong tokens when two or more requests are verified in one round: duplicated or skipped tokens, and text from other concurrent requests. Draft acceptance falls at four or more concurrent requests. A single request is unaffected, and
--mtp-draft-tokens 0is correct at every concurrency. The rule is recipe-independent, so any forward with this copy shape is exposed.Change
A complete copy becomes a writeback only when its source reads no storage region the copy writes (
mutable_keys.isdisjoint(accesses[source])). Otherwise it stays an orderedflare::storage_call, so the gather completes before the destination is written. Every other complete copy keeps fusing into Inductor's writeback. The package format is unchanged; existing packages must be re-exported to compile with this rule.Tests:
tests/test_compilation.py::test_row_expansion_reads_overlapping_rows_before_writing(CUDA): compiles and replays a prefix-row expansion into the same buffer and compares exactly with eager.tests/preparation/test_invocation_views.py::test_copy_lowering_preserves_destination_for_sources_gathered_from_it(CPU): an overlapping gather stays an ordered copy; a gather into a separate buffer remains a writeback.Validation
test_compilation.py,test_forward_compile_options.py,test_invocation_views.pyon GB10bf16_add_outnative replay fails with and without this change because the installed Heliopause lacks thequalificationfeaturePackage validation conditions: TP4 across four DGX Spark hosts over RoCE, 8192-token prefill chunk, speculation enabled with adaptive verification. Other benchmark cells (concurrency 1, 2 and 8; prefill; coding) stay within run-to-run variation of the package compiled without this change.
Limitations