Skip to content

PCP + ring attention on top of DCP batched-RPA (#3255) - #3381

Closed
wenxindongwork wants to merge 12 commits into
mainfrom
wxd-pcp-ring-batched-rpa
Closed

PCP + ring attention on top of DCP batched-RPA (#3255)#3381
wenxindongwork wants to merge 12 commits into
mainfrom
wxd-pcp-ring-batched-rpa

Conversation

@wenxindongwork

Copy link
Copy Markdown
Collaborator

Description

Stacked PR: PCP + ring attention (#3277) rebased on top of the DCP batched-RPA PR (#3255).

This branch is feat/dcp-on-batched-rpa (#3255, @weiyu0824) with the in-kernel PCP ring attention from #3277 merged on top, so the two features can land together without conflicts. Please review only the merge commit; the underlying changes are reviewed in their own PRs.

What the merge reconciles

Both PRs rewrite parts of tpu_inference/layers/common/cp_attention.py:

Everything else merged cleanly: #3277's changes live in rpa_v3_cp/kernel.py + sharding.py, #3255's in batched_rpa/.

Not in scope

Porting the ring into the batched RPA kernel itself. The batched kernel fetches KV through emit_pipeline buffered refs driven by precomputed DMA descriptors (schedule.py / bref_override.py), so an in-kernel ring there needs a ring-aware KV buffered ref and schedule rounds — a follow-up, not a merge.

Tests

On a v7x-8 (worktree code first on PYTHONPATH):

  • python tests/kernels/rpa_v3_cp/ragged_paged_attention_kernel_cp_test.py RaggedPagedAttentionPcpTest — 30/30 pass, including the 6 test_pcp_ring_cache_phase_matches_full_cache variants (P=2/4/8, fp32/bf16).
  • python tests/layers/common/test_pcp_attention_interface.py — 16/16 pass.
  • USE_BATCHED_RPA_KERNEL=1 python tests/kernels/rpa_v3_cp/ragged_paged_attention_kernel_dcp_test.py — both tests error with RuntimeError: Array has been deleted when the test re-reads the donated kv_cache input. The pristine feat/dcp-on-batched-rpa tree fails identically in the same environment, so this is pre-existing on feat: Enable DCP for batched RPA kernel #3255 (donation vs. this jax version), not introduced by the merge.

Checklist

Before submitting this PR, please make sure:

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have made or will make corresponding changes to any relevant documentation.

weiyu0824 and others added 12 commits August 1, 2026 00:23
Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
…uarded cond in non-cp pass

Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
…on schedule and rely on existing bref_override infra

Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
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>
Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
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>
Signed-off-by: Wei-Yu Lin <weiyulin@google.com>
- 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>
Reconcile cp_attention.py: keep the USE_BATCHED_RPA_KERNEL dispatch and
AttentionScope mapping from the DCP PR, and the in-kernel ring cache
phase in pcp_forward from the ring PR. PCP under the batched RPA kernel
still raises NotImplementedError; the ring runs on the rpa_v3_cp path.
@github-actions

Copy link
Copy Markdown

Description

Start with a short description of what the PR does and how this is a change from
the past.

The rest of the description includes relevant details and context, examples:

  • why is this change being made,
  • the problem being solved and any relevant context,
  • why this is a good solution,
  • some information about the specific implementation,
  • shortcomings of the solution and possible future improvements.

If the change fixes a Github issue, please include a link, e.g.,:
FIXES: #123456

Tests

Please describe how you tested this change, and include any instructions and/or
commands to reproduce.

Checklist

Before submitting this PR, please make sure:

  • I have performed a self-review of my code.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have made or will make corresponding changes to any relevant documentation.

@wenxindongwork

Copy link
Copy Markdown
Collaborator Author

Superseded by #3382, which is based on the DCP PR's branch so the diff shows only the ring changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants