Skip to content

[Fix] attention: Keep fused RoPE KV writes off DCP layouts - #47261

Merged
BBuf merged 1 commit into
sgl-project:mainfrom
LLLL114:fix/dcp-fused-kv-store
Nov 3, 2026
Merged

BBuf merged 1 commit into
sgl-project:mainfrom
LLLL114:fix/dcp-fused-kv-store

Conversation

@LLLL114

@LLLL114 LLLL114 commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Qwen3-30B-A3B with TP8/DCP2, Triton attention and BF16 KV can write DCP logical slot IDs directly into a rank-local KV buffer through fused RoPE + KV storage. This causes incorrect output and can produce CUDA illegal memory accesses as the cache allocation advances.

Modifications

enable_fused_set_kv_buffer() checks forward_batch.dcp_kv_mask to avoid DCP, but eager batch construction only populates that mask on HIP. CUDA batches can therefore have DCP enabled with a None mask.

Qwen3 MoE then selects fused RoPE storage, whose FusedSetKVBufferArg.cache_loc is the unchanged out_cache_loc, and skips the attention backend's KV store. That bypasses the Triton DCP path's owner mask and loc // dcp_size translation.

  • Reject the generic fused RoPE KV-store path when get_parallel().dcp_enabled is true. The existing attention backend performs the DCP-aware store.
  • Add a regression to the existing gate test for CUDA/HIP, DCP1/2/4, and an absent batch mask. Keep DCP1 eligible for fusion.

Accuracy Tests

  • All 1,319 official GSM8K test examples, in dataset order.
  • Qwen3-30B-A3B, complete real BF16 weights; 8 × NVIDIA A10; TP8/DCP2; Triton attention; BF16 KV;
  • Chat API, 5-shot prompting, enable_thinking=False, temperature 0, top_p=1, max_tokens=1024, server seed 42.

Results:

Metric Main Main + fix
Correct / scored 13 / 1,319 1,232 / 1,319
Accuracy 0.99% 93.40%

Speed Tests and Profiling

Checklist

Review and Merge Process

  1. Ping Merge Oncalls to start the process. See the PR Merge Process.
  2. Get approvals from CODEOWNERS and other reviewers.
  3. Trigger CI tests with comments or contact authorized users to do so.
    • Common commands include /tag-and-rerun-ci, /tag-run-ci-label, /rerun-failed-ci
  4. After green CI and required approvals, ask Merge Oncalls or people with Write permission to merge the PR.

CI States

Latest PR Test (Base): ❌ Run #40868084920
Latest PR Test (Extra): ❌ Run #40868084748
Latest PR Test (AMD ROCm 10): ✅ Run #40868084788

Tip

Base CI only covers the core cases, and its workload may not exercise your change. Use /rerun-test to run the tests your change touches, or copy this prompt for an agent:

Find the registered tests this PR affects and run them with /rerun-test, following https://github.com/sgl-project/sglang/blob/main/.agents/skills/rerun-test/SKILL.md

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@BBuf BBuf left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[BBuf Review Agent] LGTM. Please wait for the related CI to pass.

@BBuf

BBuf commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Nov 2, 2026
@BBuf
BBuf merged commit f6db18e into sgl-project:main Nov 3, 2026
256 of 285 checks passed
@LLLL114
LLLL114 deleted the fix/dcp-fused-kv-store branch November 3, 2026 03:42
Yuxingwang-intel pushed a commit to Yuxingwang-intel/sglang that referenced this pull request Nov 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants