Skip to content

[ROCm][Perf] Tune MiniMax-M3 decode top-k for short contexts - #55235

Merged
AndreasKaratzas merged 4 commits into
vllm-project:mainfrom
Fangzhou-Ai:afz/mm3-rocm-short-topk
Sep 14, 2026
Merged

AndreasKaratzas merged 4 commits into
vllm-project:mainfrom
Fangzhou-Ai:afz/mm3-rocm-short-topk

Conversation

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator

Summary

  • Use one B128/C1/W2/S1 CTA for the gfx950 MiniMax-M3 decode selector when topk == 16 and the graph capacity is at most 128 sparse blocks.
  • Keep the existing launch policy unchanged for larger capacities, other top-k values, and other architectures.
  • Extend the selector tests to cover the production 74-block capacity, the 128/129 dispatch boundary, exact ordering, and graph replay.

Kernel performance

Measured on gfx950 at the production 8k1k graph capacity of 74 blocks, with query length 4, one local index head, top-k 16, and the recorded c1/c2/c4/c6/c8/c12/c16/c24/c32/c64 input rows:

Stage Existing B512/C16 B128/C1 Improvement
Fused selector + sparse table 4.138-4.440 us 2.947-3.213 us 26.73-28.80%
BF16 score + selector + sparse table 7.560-24.469 us 6.338-23.158 us 5.36-16.16%

Each timing sample used a 64-node HIP graph. Every case had five seconds of same-workload graph warmup, 16 full-graph pre-replays before each sample, and 31 balanced HIP-event samples.

A separate exact boundary sweep over capacities 65, 74, and 128; query lengths 1, 4, and 8; and local index-head counts 1, 2, and 4 measured 32.78-35.32% lower selector time and 26.19-29.21% lower selector-plus-table time.

Precision

This changes launch geometry only. The BF16 index cache, FP32 score output, score arithmetic, and deterministic score-descending/index-ascending ordering are unchanged. Tests matched an independent CPU total-order reference exactly, including NaN, infinities, signed zero, and tied boundary scores. Top-k IDs, sparse tables, context lengths, completion-counter reset, eager execution, and graph replay all matched exactly.

This PR preserves the current vLLM BF16 precision contract; it does not switch the indexer to the separate AITER FP8 path.

Validation

python -m pytest tests/kernels/attention/test_minimax_m3.py -q
121 passed, 13 skipped

pre-commit run --files vllm/models/minimax_m3/amd/ops/index_topk.py tests/kernels/attention/test_minimax_m3.py
all applicable hooks passed

No model evaluation was rerun because the change is output-exact and does not alter model arithmetic or precision.

Duplicate-work check

This is a narrow launch-policy follow-up to merged #54682. The required open-PR searches found no PR tuning this fused BF16 decode selector to B128/C1/W2/S1. #53448 targets prefill plus an optional FP8 index cache, #52664 integrates an external AITER indexer path, and #52059 targets prefill scoring; none makes this decode launch-policy change.

OpenAI Codex assisted with analysis, implementation, benchmarking, validation, and drafting this description. The human submitter must review every changed line and be able to reproduce and defend the change.

Use a single 128-entry CTA for the gfx950 BF16 decode selector when the graph capacity is at most 128 sparse blocks.

Co-authored-by: OpenAI Codex <codex@openai.com>

Signed-off-by: fai <fangzhouai@gmail.com>

@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.

@mergify mergify Bot added minimax rocm Related to AMD ROCm labels Sep 3, 2026
@github-project-automation github-project-automation Bot moved this to Todo in AMD Sep 3, 2026
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 0dcbca20-e566-4811-bd7b-1a857813e413

📥 Commits

Reviewing files that changed from the base of the PR and between e962733 and be341d6.

📒 Files selected for processing (2)
  • tests/kernels/attention/test_minimax_m3.py
  • vllm/models/minimax_m3/amd/ops/index_topk.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • vllm/models/minimax_m3/amd/ops/index_topk.py
  • tests/kernels/attention/test_minimax_m3.py

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Performance Improvements
    • Improved AMD ROCm decode top-k execution by selecting kernel block sizes, warp counts, and staging based on workload characteristics.
    • Added optimized handling for short-context top-k workloads and refined dispatch behavior at key size boundaries.
    • Improved execution across a wider range of sequence lengths, including workloads near dispatch thresholds.
    • Added graph-replay coverage for fused top-k execution.

Walkthrough

The AMD decode top-k policy now selects block size, warp count, and stage count. The fused kernel receives these values. Tests cover short-context boundaries, parameterized sequence sizes, ordering, and graph replay.

Changes

AMD decode top-k dispatch

Layer / File(s) Summary
Launch policy specialization
vllm/models/minimax_m3/amd/ops/index_topk.py
The policy returns parameters for short-context, standard gfx950, and generic fallback configurations.
Kernel wiring and boundary validation
vllm/models/minimax_m3/amd/ops/index_topk.py, tests/kernels/attention/test_minimax_m3.py
The decode path forwards selected parameters to the fused kernel. Tests cover dispatch boundaries, parameterized sequence lengths, ordering, and graph replay.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to be341

This change specializes the gfx950 short-context BF16 decode top-k launch configuration while retaining existing policies elsewhere; no concrete merge-blocking risk is identified.

Sequence Diagram(s)

sequenceDiagram
  participant Decode as minimax_m3_index_decode
  participant Policy as _decode_topk_launch_policy
  participant Kernel as _decode_topk_fused_kernel
  Decode->>Policy: Request top-k launch configuration
  Policy-->>Decode: Return block size, warps, stages, chunk count, and flags
  Decode->>Kernel: Launch with selected configuration
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: performance tuning for MiniMax-M3 decode top-k on ROCm, with emphasis on short contexts.
Description check ✅ Passed The description is directly related to the changeset. It explains the new launch policy, preserved behavior, test coverage, benchmark results, and validation performed.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
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.

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.

@AndreasKaratzas AndreasKaratzas added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 3, 2026
@AndreasKaratzas

Copy link
Copy Markdown
Member

/ci run

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

@Fangzhou-Ai, CI is now available for this PR.

  • /ci run starts upstream CI; /amd-ci run starts AMD CI only.
  • /ci retry retries failed jobs in the CI build for the current PR head. If the current head has no CI build, it starts a new CI build for the current head containing only jobs that failed in the latest earlier CI build for this PR.
  • /amd-ci retry retries failed jobs in AMD CI for the current PR head. Use /amd-ci run when the current head has no AMD CI build.
  • /ci cancel cancels scheduled or running CI builds for this PR branch; /amd-ci cancel does the same for AMD CI only.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87142 for commit 70746a61db59.

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87276 for commit 3091c785afcd.

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator Author

/ci run

@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown

✅ Triggered Buildkite CI #87371 for commit be341d6e3586.

@zhou9402

zhou9402 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

LGTM

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator Author

/ci run

@github-actions

Copy link
Copy Markdown

✅ Triggered Buildkite CI #88455 for commit 2ab9b5723c48.

@Fangzhou-Ai

Copy link
Copy Markdown
Collaborator Author

hi @zhou9402 can we merge it now?

@AndreasKaratzas
AndreasKaratzas merged commit dfa1984 into vllm-project:main Sep 14, 2026
84 checks passed
@github-project-automation github-project-automation Bot moved this from Todo to Done in AMD Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minimax ready ONLY add when PR is ready to merge/full CI is needed rocm Related to AMD ROCm

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants