Skip to content

Add chunking strategy for fp8_paged_mqa_logits - #398

Draft
Xia-Weiwen wants to merge 5 commits into
mainfrom
fp8_mqa_logits_oom
Draft

Add chunking strategy for fp8_paged_mqa_logits#398
Xia-Weiwen wants to merge 5 commits into
mainfrom
fp8_mqa_logits_oom

Conversation

@Xia-Weiwen

Copy link
Copy Markdown
Collaborator

export SGL_KERNEL_FP8_PAGED_MQA_CHUNK_MB=512 (default is 512)

@polisettyvarma polisettyvarma left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

helps in OOM and which model, right ?

@Xia-Weiwen

Copy link
Copy Markdown
Collaborator Author

helps in OOM and which model, right ?

It can probably resolve the OOM issue of DeepSeek. Jianan is going to have a try.

@Xia-Weiwen
Xia-Weiwen marked this pull request as ready for review August 25, 2026 05:08
@Xia-Weiwen
Xia-Weiwen requested review from airMeng and mingfeima and a lite review from Copilot August 25, 2026 05:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds configurable batch chunking to the Xe20 FP8 paged MQA logits GEMM path.

Changes:

  • Adds a configurable 512 MiB default chunk budget.
  • Processes batches with chunk-local intermediates.
  • Adds optional verbose chunk diagnostics.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/sycl/Fp8MqaLogitsXe20.cpp Outdated
Comment thread src/sycl/Fp8MqaLogitsXe20.cpp
" chunks)");
}

for (int start = 0; start < B_next; start += chunk_b) {

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.

avoid launching kernels in the loop, fuse into a larger kernel instead

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the suggestion. My agent says the loop introduces limited kernel launch overhead (~2% in the DeepSeek V4 case). I have added a TODO here to indicate an opportunity for optimizations. I think we can keep the current implementation to resolve the OOM issue which is blocking.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

After an offline discussion, we decide to improve the kernel implementation for better performance so I have converted this PR to draft and will work on it later. Thanks.

@Xia-Weiwen
Xia-Weiwen marked this pull request as draft August 26, 2026 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants