Skip to content

dcp: make head-sliced attention views contiguous for the B12X PCIe pool - #81

Merged
lukealonso merged 1 commit into
dev/eldritch-enlightenmentfrom
fable/dcp-b12x-contiguous-lse-20260707
Jul 14, 2026
Merged

lukealonso merged 1 commit into
dev/eldritch-enlightenmentfrom
fable/dcp-b12x-contiguous-lse-20260707

Conversation

@voipmonitor

@voipmonitor voipmonitor commented Jul 7, 2026

Copy link
Copy Markdown

Problem

TP6 + DCP>1 with the B12X A2A fast path dies at CUDA graph capture:

ValueError: partial_lse must be contiguous
  pool.lse_reduce_scatter(cp_attn_out, cp_attn_lse, ...)   # pcie_dcp_a2a._validate

Root cause: GLM TP6 virtual-TP pads attention heads 64 → 66, and the B12X sparse MLA kernel pads its head dim further, returning the decode output/LSE as head-sliced views (b12x_mla_sparse.py: lse = lse[:, :input_num_heads]) — non-contiguous whenever kernel head count > input head count. TP8 shapes never hit this (counts match), which is why all DCP2/DCP4 runs to date were fine. The NCCL packers take explicit stride arguments and the query all-gather wrapper already calls .contiguous(); only the PCIe pool's lse-reduce entry lacked the fix-up.

Fix

Copy the operands to contiguous in _try_b12x_dcp_lse_reduce, placed after the cheap reject checks so NCCL fallback paths pay nothing. The LSE copy is a tiny [B, H] fp32; the output copy only happens on padded head counts. Graph-capture safe (same in-graph allocation pattern as the packed A2A send/recv buffers).

Matches the minimal patch Martin drafted (blackwell-llm-docker/patches/vllm-dcp-b12x-contiguous-lse-20260707.patch) — this PR adds the placement inside the try-helper plus a unit test that simulates the sliced-view pattern and asserts the pool receives contiguous tensors (31/31 in test_dcp_a2a.py on the v6 image, distributed cases deselected).

Verification

Diagnosed on TP6/MXFP4/A8/DCP2, v6 image (vllm49bed029-b12x26144c0): previously died at capture; E2E with this file mounted boots and serves (numbers in comment below).

Stacked on fable/w4a8mx-qmma-pad-20260707 (#80) — the v6 image pin.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved distributed attention processing for head-sliced tensors.
    • Ensured partial attention outputs and LSE data are contiguous before reduction, preventing failures with non-contiguous inputs.
  • Tests
    • Added CUDA coverage validating correct handling of non-contiguous attention tensor views.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: d6e8baca-16ad-497c-82c6-130d613eae0d

📥 Commits

Reviewing files that changed from the base of the PR and between dc47f72 and e95da31.

📒 Files selected for processing (2)
  • tests/distributed/test_dcp_a2a.py
  • vllm/v1/attention/ops/dcp_alltoall.py

📝 Walkthrough

Walkthrough

The B12X PCIe DCP LSE reduction path now makes non-contiguous attention output and LSE views contiguous before dispatch. A CUDA-only test constructs non-contiguous slices and verifies the pool receives contiguous tensors.

Changes

B12X LSE contiguity handling

Layer / File(s) Summary
Contiguous reduction dispatch and validation
vllm/v1/attention/ops/dcp_alltoall.py, tests/distributed/test_dcp_a2a.py
_try_b12x_dcp_lse_reduce copies non-contiguous output and LSE tensors before lse_reduce_scatter; the CUDA test verifies this behavior with padded tensor views.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making head-sliced attention views contiguous for the B12X PCIe pool.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fable/dcp-b12x-contiguous-lse-20260707

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@voipmonitor

Copy link
Copy Markdown
Author

E2E verified on the exact failing config — v6 image (vllm49bed029-b12x26144c0), GLM-5.2 AMD MXFP4 experts, TP6 / force-A8 / online MXFP8 dense / DCP2, GMU 0.950, with only this PR's dcp_alltoall.py mounted:

  • previously: died at CUDA graph capture with ValueError: partial_lse must be contiguous
  • now: boots through capture, KV cache 639,616 tokens, coding smoke c0 = 67.1 tok/s and c3000 = 66.9 tok/s, both 0 CJK (i.e. the copied views also produce correct DCP-reduced outputs, not just a passing boot)

Martin's build patch blackwell-llm-docker/patches/vllm-dcp-b12x-contiguous-lse-20260707.patch is content-equivalent to this PR (same guarded copies, same placement) — safe to unblock the image build with it; this PR adds the explanatory comment and the unit test.

@voipmonitor
voipmonitor changed the base branch from fable/w4a8mx-qmma-pad-20260707 to dev/eldritch-enlightenment July 7, 2026 14:15
@voipmonitor
voipmonitor force-pushed the fable/dcp-b12x-contiguous-lse-20260707 branch from e2e2eaf to c2d0828 Compare July 7, 2026 19:43
Sparse MLA backends can return the decode output and LSE as head-sliced
views: GLM TP6 virtual-TP pads 64 -> 66 attention heads, the B12X sparse
kernel pads its head dim further and slices the result back
(b12x_mla_sparse lse = lse[:, :input_num_heads]), which is non-contiguous
whenever the kernel head count exceeds the input head count. The B12X
PCIe DCP pool validates contiguity, so TP6 + DCP>1 with the A2A fast
path died at CUDA graph capture with 'partial_lse must be contiguous'.
TP8 shapes never hit this because kernel and input head counts match.

Copy the operands to contiguous in _try_b12x_dcp_lse_reduce, after the
cheap reject checks so NCCL fallback paths pay nothing. The NCCL packers
take explicit strides and never needed this; the query all-gather wrapper
already does the same for its input. The LSE copy is [B, H] fp32 and the
output copy only happens on padded head counts.

Diagnosed by Martin on TP6/MXFP4/A8/DCP2 (v6 image); unit test simulates
the sliced-view pattern and asserts the pool receives contiguous tensors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@voipmonitor
voipmonitor force-pushed the fable/dcp-b12x-contiguous-lse-20260707 branch from c2d0828 to e95da31 Compare July 9, 2026 22:57
@voipmonitor

Copy link
Copy Markdown
Author

Rebased to a single commit on current dev/eldritch-enlightenment (post #78/#79/#83 merges — those landed while this one was skipped, which had pushed it into conflict). mergeable=true again; suite run on the v6 image with this file mounted: 31 passed + the same 1 pre-existing environment-bound failure (TestDCPCommBackendConfig::test_a2a_with_dcp_valid) that pure dev shows in the same container.

Still needed: dev has no equivalent fix — the sparse-MLA head-sliced LSE views reach the PCIe pool non-contiguous on TP6×DCP≥2 and capture dies with partial_lse must be contiguous. Production images only survive because the content-equivalent build patch (blackwell-llm-docker/patches/vllm-dcp-b12x-contiguous-lse-20260707.patch) rides along; merging this retires that patch.

@lukealonso
lukealonso merged commit 20904b6 into dev/eldritch-enlightenment Jul 14, 2026
2 of 3 checks passed
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