PCP + ring attention - #3277
Open
wenxindongwork wants to merge 5 commits into
Open
Conversation
DescriptionStart with a short description of what the PR does and how this is a change from The rest of the description includes relevant details and context, examples:
If the change fixes a Github issue, please include a link, e.g.,: TestsPlease describe how you tested this change, and include any instructions and/or ChecklistBefore submitting this PR, please make sure:
|
wenxindongwork
force-pushed
the
wxd-pcp-ring-attn
branch
from
July 29, 2026 01:01
9631f6e to
327974f
Compare
wenxindongwork
force-pushed
the
wxd-pcp-gatherkv
branch
from
July 31, 2026 05:45
9830975 to
833b596
Compare
wenxindongwork
force-pushed
the
wxd-pcp-ring-attn
branch
from
July 31, 2026 18:56
327974f to
f9a7a91
Compare
wenxindongwork
force-pushed
the
wxd-pcp-ring-attn
branch
from
July 31, 2026 20:47
f9a7a91 to
a98bb16
Compare
wenxindongwork
force-pushed
the
wxd-pcp-ring-attn
branch
from
August 7, 2026 23:44
7af52ac to
2a70feb
Compare
Adds a third cache-phase strategy for prefill context parallelism:
instead of all-gathering Q (and reduce-scattering the output) or
all-gathering the striped KV cache, the kernel rotates the cache
around the PCP ring one block at a time with in-kernel remote DMAs,
attending each block as it arrives. (m, l, acc) accumulate across all
rounds, so the whole cache folds into one online softmax with no
output collective and only two KV blocks resident.
Implementation: process() drives the ring by scaling its KV-block loop
by cp_group_size -- each iteration is one round of one block, falling
through to the shared flash-attention code with the slot and the
originating rank's stripe length swapped in. Prefetch, waits, and
sizing are the plain path's own, conditioned internally. Rotation uses
one DMA semaphore pair plus a single credit semaphore ("the slot you
are about to overwrite is free"), with the block barrier expressed as
the last round's credit; launch-first/launch-last predicates keep the
ledger balanced at zero. Ring block sizing lives in
get_default_block_sizes: one Q block per launch (each extra block
re-streams the whole cache), ~2MB hops to amortize the per-hop fixed
cost, bq halved until the VMEM estimate fits.
The ring wins where TP shrinks the per-device KV: at pcp4 x tp2 it is
the best strategy from 64k context up (1M cumulative TTFT 4425 vs 4861
auto), while at pcp8 x tp1 it loses to the gathers (9616 vs 4571) --
selection is left to the caller via cache_phase until an automatic
dispatch rule lands.
Verified: kernel suite (ring vs full-cache reference at P in {2,4,8},
both dtypes, plus DCP/decode/prefill/mixed regressions), interface
suite on the production 7-axis mesh, and cumulative-TTFT sweeps to 1M
on both shardings.
Signed-off-by: wenxindongwork <wenxindong@google.com>
wenxindongwork
force-pushed
the
wxd-pcp-ring-attn
branch
from
August 7, 2026 23:49
2c03f64 to
351d99f
Compare
wenxindongwork
marked this pull request as ready for review
August 8, 2026 00:35
wenxindongwork
requested review from
gxd3,
jrplatin,
kyuyeunk,
lk-chen and
weiyu0824
as code owners
August 8, 2026 00:35
weiyu0824
reviewed
Aug 10, 2026
weiyu0824
reviewed
Aug 11, 2026
wenxindongwork
marked this pull request as draft
August 12, 2026 17:17
Delete the gather_kv and gather_q cache-phase paths and the comm-volume auto-dispatch: the ring wins end-to-end at every measured input length on Qwen3-235B-A22B-FP8 at pcp2 x tp4 (256k prefill: 4552 tok/s vs 3749 gather-auto, mean TTFT 317s vs 396s; 4k: -16.5% TTFT; 1k: -2%), needs no gathered-KV HBM materialization, and has no output collective. Also clamp the ring Q tile to 8192 token-head rows: the VMEM estimator undercounts the rows x bkv QK^T scratch and the maximizer otherwise picks tiles that OOM compilation at head_dim=128 (estimator says fit, Mosaic OOMs). The clamp reproduces every benchmark-validated tile choice (512 @ 16 q-heads, 256 @ 32, 1024 @ 8). Signed-off-by: wenxindongwork <wenxindong@google.com>
Signed-off-by: wenxindongwork <wenxindong@google.com>
wenxindongwork
marked this pull request as ready for review
August 12, 2026 18:34
weiyu0824
approved these changes
Aug 12, 2026
lk-chen
approved these changes
Aug 12, 2026
Collaborator
|
@copilot show per-file owner, don't at them, just username |
Contributor
Per CODEOWNERS for the files touched here:
|
bhuvanpkaruturi
approved these changes
Aug 12, 2026
bhuvanpkaruturi
left a comment
Collaborator
There was a problem hiding this comment.
Left comments on couple of issues (better assertions and reducing VMEM wastage). Other LGTM.
- get_default_block_sizes: apply the ring tile/hop sizing only for RpaCase.MIXED (the only case the ring runs) so DECODE kernels keep their own sizing instead of an unnecessarily large bq_sz. - sharding config validation: reject odd prefill_context_parallelism at server startup instead of failing later inside the kernel. Signed-off-by: wenxindongwork <wenxindong@google.com>
wenxindongwork
requested review from
mrjunwan-lang and
sixiang-google
as code owners
August 12, 2026 22:09
wenxindongwork
disabled auto-merge
August 12, 2026 22:55
This was referenced Aug 12, 2026
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
This PR adds a third PCP cache-phase strategy —
cache_phase="ring"in addition to the existing gather_kv/gather_q strategy.gather_qgather_kvRing is now the default behavior for PCP since it outcompetes gather_q and gather_kv in almost every setting.
Implementation details
Each rank keeps its local Q and cache stripe; the kernel rotates the cache one block at a time with
make_async_remote_copy.The conceptual change vs the non-ring path is how bkv is prefetched. In the non-ring path, we keep a bkv double buffer and prefetch the next bkv block from HBM. In the ring path, we still keep a bkv double buffer, but prefetch the next block via ICI from the direct neighbor (except for the first ring step which prefetches from HBM).
We also added a custom block size calculation logic for the ring path, which maximizes bq_sz.
PCP strategy performance comparison.
E2E run (Qwen3 235b fp8)
Commands:
Validated model quality with gsm 8k