Skip to content

perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes / 为 kimik2.5-fp4-mi355x-vllm 启用 vLLM 编译期融合 pass#2264

Closed
jiacao-amd wants to merge 2 commits into
mainfrom
perf/kimik2.5-fp4-mi355x-vllm-fusion-passes
Closed

perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes / 为 kimik2.5-fp4-mi355x-vllm 启用 vLLM 编译期融合 pass#2264
jiacao-amd wants to merge 2 commits into
mainfrom
perf/kimik2.5-fp4-mi355x-vllm-fusion-passes

Conversation

@jiacao-amd

@jiacao-amd jiacao-amd commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Enable four vLLM torch.compile fusion passes on the Kimi-K2.5-MXFP4 MI355X fixed-seq-len vLLM launch (benchmarks/single_node/fixed_seq_len/kimik2.5_fp4_mi355x.sh):

  • fuse_norm_quant
  • fuse_act_quant
  • fuse_allreduce_rms
  • fuse_mla_dual_rms_norm

This script previously passed no --compilation_config at all, so these passes were off. Sibling Kimi scripts already use this pattern (kimik2.5_fp4_b300.sh, the agentic Kimi scripts use fuse_allreduce_rms). Also drops a duplicate --no-enable-prefix-caching line.

Motivation

Two-trace (mapping/formal) torch-profiler triage of Kimi-K2.5-MXFP4 @ TP8, c32, 8k/1k on MI355X showed:

  • all-reduce fully serial (excl 100% / hid 0%) in both prefill (NCCL 33.5%) and decode (7.6%) → fuse_allreduce_rms target.
  • un-collapsed residual-add+RMSNorm and RMSNorm+Quant kernel families → fuse_norm_quant / fuse_act_quant targets.

Status: DRAFT — perf to be measured on CI runner

Local iteration used an AITER-off baseline, so the local +87% decode-throughput number does not isolate the fusion-pass delta on top of the real CI config (which already sets VLLM_ROCM_USE_AITER=1). Requesting a CI run to get the authoritative comparison of this script with vs. without the fusion passes.

Local accuracy check (AITER-on, GSM8K 100-sample 2-shot greedy) showed no regression (0.9500 with fusion passes; 0.9500 AITER-off baseline).

Test plan

  • CI: run kimik2.5 fp4 mi355x vllm single-node 8k1k @ conc 4–64 (tp4/tp8) — compare throughput vs. current main. (1k1k configs were deprecated in Deprecate 1k1k benchmark configurations #2263.)
  • CI: eval-only run to confirm GSM8K/accuracy parity.
  • Confirm no server-startup regression from the added compile passes, and check the compile log shows the fusion passes actually fired (fusion counts) rather than silently no-op'ing.
  • Confirm with the kimik2.5_int4_mi355x.sh author why fuse_allreduce_rms was explicitly disabled there on the same MI355X SKU.

中文说明

概述

在 Kimi-K2.5-MXFP4 MI355X 定长(fixed-seq-len)vLLM 启动脚本(benchmarks/single_node/fixed_seq_len/kimik2.5_fp4_mi355x.sh)上启用四个 vLLM torch.compile 融合 pass:

  • fuse_norm_quant
  • fuse_act_quant
  • fuse_allreduce_rms
  • fuse_mla_dual_rms_norm

该脚本此前完全没有传 --compilation_config,所以这些 pass 都是关闭的。同系列 Kimi 脚本已在用这个写法(kimik2.5_fp4_b300.sh,以及 agentic Kimi 脚本用了 fuse_allreduce_rms)。同时删掉了一处重复的 --no-enable-prefix-caching

动机

对 Kimi-K2.5-MXFP4 @ TP8、c32、8k/1k 在 MI355X 上做的双 trace(mapping/formal)torch-profiler 分析显示:

  • all-reduce 完全串行(excl 100% / hid 0%),prefill(NCCL 33.5%)和 decode(7.6%)都存在 → 对应 fuse_allreduce_rms
  • 未折叠的 residual-add+RMSNormRMSNorm+Quant kernel 族 → 对应 fuse_norm_quant / fuse_act_quant

状态:DRAFT —— 性能待 CI runner 实测

本地迭代用的是 AITER 关闭的 baseline,所以本地 +87% 的 decode 吞吐数字不能单独归因于融合 pass 在真实 CI 配置(已开 VLLM_ROCM_USE_AITER=1)之上的增益。因此请求跑 CI,拿到这个脚本开/关融合 pass 的权威对比。

本地精度校验(AITER 开,GSM8K 100 样本 2-shot greedy)无回退(开融合 pass 为 0.9500;AITER 关闭 baseline 也是 0.9500)。

测试计划

  • CI:跑 kimik2.5 fp4 mi355x vllm single-node 8k1k @ conc 4–64(tp4/tp8),与当前 main 对比吞吐。(1k1k 配置已在 Deprecate 1k1k benchmark configurations #2263 废弃。)
  • CI:eval-only 跑一遍,确认 GSM8K/精度持平。
  • 确认新增的编译 pass 不引入 server 启动回退,并检查 compile 日志显示融合 pass 真正生效(有 fusion counts),而不是静默 no-op。
  • kimik2.5_int4_mi355x.sh 的作者确认:为什么在同样的 MI355X SKU 上那个脚本显式禁用了 fuse_allreduce_rms

Add fuse_norm_quant / fuse_act_quant / fuse_allreduce_rms /
fuse_mla_dual_rms_norm to the vLLM launch, matching the pattern already
used by sibling Kimi scripts (kimik2.5_fp4_b300.sh and the agentic Kimi
scripts). The mi355x fixed-seq-len script previously set no
--compilation_config at all, so these fusion passes were off.

Motivation: two-trace (mapping/formal) torch-profiler triage of
Kimi-K2.5-MXFP4 @ TP8 c32 showed all-reduce running fully serial
(hid=0%) and an un-collapsed residual-add+RMSNorm / RMSNorm+Quant
family. fuse_allreduce_rms targets the serial all-reduce; the norm/quant
passes collapse the RMSNorm+Quant family.

Also drops a duplicate --no-enable-prefix-caching line.

Perf is to be measured on the CI runner (draft PR). Local numbers so far
were captured against an AITER-off baseline, so they do not isolate the
fusion-pass delta on top of the CI config (which already sets
VLLM_ROCM_USE_AITER=1); CI will provide the authoritative comparison.
@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs


感谢你的贡献!请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md,然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人,你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@jiacao-amd

Copy link
Copy Markdown
Collaborator Author

/sweep test-config --config-files configs/amd-master.yaml --config-keys kimik2.5-fp4-mi355x-vllm

@jiacao-amd
jiacao-amd marked this pull request as ready for review July 18, 2026 02:34
@jiacao-amd
jiacao-amd requested a review from a team July 18, 2026 02:34
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jiacao-amd's task in 4m 55s —— View job


PR Review

  • Gather context (diff, script, sibling scripts, configs, AGENTS.md conventions)
  • Review changes
  • Post review feedback (2 inline comments)

Summary

The change itself is clean — the dotted --compilation_config.pass_config.* flag syntax matches the sibling Kimi scripts exactly, and removing the duplicated --no-enable-prefix-caching is correct. Two issues found:

  1. 🔴 Missing perf-changelog.yaml entry — this is a perf-affecting recipe change, and without an appended changelog entry the kimik2.5-fp4-mi355x-vllm config won't be re-benchmarked on main after merge (see inline comment for a ready-to-paste entry).
  2. 🟡 First ROCm use of these fusion passes — the only other AMD script referencing fuse_allreduce_rms (kimik2.5_int4_mi355x.sh, same MI355X SKU) explicitly disables it, and the siblings cited as precedent are all NVIDIA; fuse_norm_quant / fuse_act_quant / fuse_mla_dual_rms_norm have no prior usage in the repo at all. The planned CI sweep + eval run will validate startup/accuracy, but please confirm the passes actually fire (not silently no-op) and why the int4 sibling opted out.

Note: the PR description references 1k/1k measurements, but 1k1k configs were deprecated in #2263 — the active sweep for this config is 8k/1k (tp4/tp8, conc 4–64).

…asses

Required changelog entry for PR #2264. Appended at end of file per
repo convention. This also enables the sweep CI (run-sweep.yml triggers
on perf-changelog.yaml changes).
Comment on lines +77 to +80
--compilation_config.pass_config.fuse_norm_quant true \
--compilation_config.pass_config.fuse_act_quant true \
--compilation_config.pass_config.fuse_allreduce_rms true \
--compilation_config.pass_config.fuse_mla_dual_rms_norm true \

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.

🔴 BLOCKING: Missing perf-changelog.yaml entry for a perf-affecting recipe change.
Why it matters: Per AGENTS.md, perf-changelog.yaml is what triggers benchmark runs for a config after merge. Without an entry, kimik2.5-fp4-mi355x-vllm won't be re-benchmarked on main and the dashboard keeps the pre-fusion numbers. Every comparable change to this config (e.g. the v0.22.0→v0.24.0 image bump in #2074, the concurrency-sweep changes in #1674/#1677) shipped with an entry.
Fix: Append (never insert mid-file, preserve whitespace) to the end of perf-changelog.yaml:

- config-keys:
    - kimik2.5-fp4-mi355x-vllm
  description:
    - "Enable vLLM compile-time fusion passes: fuse_norm_quant, fuse_act_quant, fuse_allreduce_rms, fuse_mla_dual_rms_norm"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2264

@jiacao-amd jiacao-amd Jul 18, 2026

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.

Done

--no-enable-prefix-caching \
--compilation_config.pass_config.fuse_norm_quant true \
--compilation_config.pass_config.fuse_act_quant true \
--compilation_config.pass_config.fuse_allreduce_rms true \

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.

🟡 WARNING: This is the first ROCm enablement of fuse_allreduce_rms in this repo — and the only other AMD reference, kimik2.5_int4_mi355x.sh:46 on the same MI355X SKU, explicitly disables it (--compilation-config '{"pass_config": {"fuse_allreduce_rms": false}}'). The sibling scripts cited in the PR description (kimik2.5_fp4_b300.sh, kimik2.5_int4_b200.sh, agentic int4_h200, etc.) are all NVIDIA.
Why it matters: If the int4 script's opt-out was working around a ROCm crash/accuracy/perf issue with the fused allreduce+RMSNorm path, this flag could regress rather than help — especially combined with VLLM_ROCM_QUICK_REDUCE_QUANTIZATION=INT4 (line 49), whose quantized custom all-reduce may not compose with the fused collective op. Note also that fuse_norm_quant, fuse_act_quant, and fuse_mla_dual_rms_norm have no prior usage anywhere in this repo; if any name is invalid in the vllm/vllm-openai-rocm:v0.24.0 image, PassConfig validation rejects it and the server fails to start.
Fix: The planned CI sweep + eval run (already in your test plan) covers this — please also confirm with the int4 script's author why fuse_allreduce_rms was disabled there before merging, and check the server log shows the passes actually fired (fusion counts in the compile logs) rather than silently no-op'ing.

Comment on lines 74 to 83
--block-size=1 \
--no-enable-prefix-caching \
--trust-remote-code \
--no-enable-prefix-caching \
--compilation_config.pass_config.fuse_norm_quant true \
--compilation_config.pass_config.fuse_act_quant true \
--compilation_config.pass_config.fuse_allreduce_rms true \
--compilation_config.pass_config.fuse_mla_dual_rms_norm true \
--mm-encoder-tp-mode data > $SERVER_LOG 2>&1 &

SERVER_PID=$!

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.

🟡 PR #2264's title and description are English-only, violating AGENTS.md's mandatory bilingual convention (title must be <English title> / <中文标题>, and the body must include a ## 中文说明 section mirroring the summary).

Extended reasoning...

AGENTS.md line 7 states, in mandatory language: "PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format: <English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a ## 中文说明 section mirroring the summary)..." It goes on to say "This applies to every PR and every issue."

This PR's title is only perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes — there is no / <中文标题> suffix. Likewise, the body contains Summary / Motivation / Status / Test plan sections entirely in English, with no accompanying ## 中文说明 (or equivalent) Chinese section.

Step-by-step verification:

  1. Read AGENTS.md line 7 in this repo's checkout — it explicitly mandates bilingual titles/descriptions for every PR, with a concrete title format and a suggested ## 中文说明 body section.
  2. Read the PR perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes / 为 kimik2.5-fp4-mi355x-vllm 启用 vLLM 编译期融合 pass #2264 metadata provided in context — the title has no Chinese portion, and none of the four description sections (Summary, Motivation, Status, Test plan) have a Chinese counterpart.
  3. Compare against the rule's only stated exceptions (CODEOWNER sign-off template, bot-generated comments) — neither applies to a PR author's own title/description.
  4. Conclusion: the PR as submitted violates the checked-in bilingual convention.

This is not a subjective style nit invented by the reviewer — it is a literal, checked-in repository rule (AGENTS.md, loaded via CLAUDE.md) using 'must' language and covering titles, descriptions, and even PR comments. Existing repo tooling (e.g. the CODEOWNER sign-off bot comment on this very PR) already posts bilingual English/Chinese text, showing the convention is actively enforced elsewhere in the repo's workflow, even though there's no automated CI gate for the PR title/body itself.

Fix: rename the PR title to perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes / 启用 vLLM 编译期融合优化 (or similar), and append a ## 中文说明 section to the description mirroring the Summary/Motivation/Status/Test plan content in Simplified Chinese.

Why nit and not normal: this has zero effect on the benchmark script's correctness or runtime behavior — nothing breaks, no incorrect results, no crash. It's a documentation/process compliance gap that's trivial to fix by editing the PR title and body, so it doesn't justify blocking merge, but per the repo's explicit convention it should still be raised rather than silently ignored.

@jiacao-amd jiacao-amd Jul 18, 2026

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.

Fixed

@jiacao-amd jiacao-amd changed the title perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes perf(kimik2.5-fp4-mi355x): enable vLLM compile-time fusion passes / 为 kimik2.5-fp4-mi355x-vllm 启用 vLLM 编译期融合 pass Jul 18, 2026
@jiacao-amd

Copy link
Copy Markdown
Collaborator Author

Closing: CI (8k1k, the only benchmarked workload for this config after #2263 deprecated 1k1k) shows the fusion passes give ~0.2% throughput (within noise), because their benefit is in the decode-heavy 1k1k path which is no longer swept. Re-profiling on 8k1k to find the real opportunities for this workload.

@github-actions

Copy link
Copy Markdown
Contributor

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

Projects

Development

Successfully merging this pull request may close these issues.

1 participant