Skip to content

[HIP] [Feature] Add routed GLM-5.2 TP4 MXFP4 MoE shape - #5451

Open
pbkowalski wants to merge 2 commits into
ROCm:mainfrom
pbkowalski:pr/glm52-routed-mxmoe
Open

pbkowalski wants to merge 2 commits into
ROCm:mainfrom
pbkowalski:pr/glm52-routed-mxmoe

Conversation

@pbkowalski

@pbkowalski pbkowalski commented Sep 11, 2026 •

Copy link
Copy Markdown

[Feature] Add routed GLM-5.2 TP4 MXFP4 MoE shape

Summary

Add the routed-only GLM-5.2 TP4 shape to the MXFP4 MoE auxiliary generator and
provide tuned decode configurations for gfx950.

Motivation

GLM-5.2 has 256 routed experts and one shared expert. When the shared expert is
run separately, the MXFP4 MoE call uses 256 experts, top-k 8, hidden size 6,144,
and intermediate size 512.

AITER currently generates the auxiliary sorting code for the combined
257-expert, top-k 9 shape, but not for this routed-only shape. It also has no
matching decode rows in the GLM-5.2 MXFP4 tuning table.

Changes

  • Add (256, 6144, 512, 8) to the MXFP4 MoE auxiliary generator.
  • Add gfx950 tuning rows for token buckets 2, 4, 16, 32, and 64.
  • Use the existing inline-FP4 stage 1 and atomic stage 2 kernels selected during
    tuning.
  • Extend the existing MoE sorting test with an output_aux check for the routed
    shape.

Performance

Hardware: AMD MI355X, GLM-5.2-MXFP4, TP4.

With the shared expert run separately, output throughput changed as follows:

Workload Before After Change
8k input / 1k output, concurrency 16 598.8 654.0 +9.2%
8k input / 1k output, concurrency 32 830.7 912.6 +9.9%
60k input / 600 output, concurrency 4 112.6 116.7 +3.7%
60k input / 600 output, concurrency 16 160.6 166.1 +3.4%

At 60k input and concurrency 64, throughput was unchanged within measurement
noise: 181.3 before and 181.2 after.

Testing

  • The existing MoE sorting test runs the generated auxiliary sorter for 256
    experts and top-k 8, then checks token IDs, expert IDs, row indices, weights,
    and the output buffer.
  • Existing tuning tests check the merged configuration table and execute its
    rows.
  • The integrated GLM-5.2 image completed the full serving sweep, GPQA, and
    needle-in-a-haystack tests.
python3 op_tests/test_moe_sorting.py \
  -d bf16 -m 4 -e 256 -t 8 -md 6144 -id 512 \
  -p t -dp 0 -em f -accum t

Documentation

No user-facing documentation changes are needed.

Dependencies

No new dependencies.

Breaking Changes

None.

@github-actions

Copy link
Copy Markdown
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests on MI35X (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:gfx1250-ffm-triton Run the five-shard gfx1250 FFM Triton test suite
ci:triton-300x Run an additional Triton test job on MI300X in PRs; main branch always runs both MI35X and MI300X
multigpu Aiter multi-GPU tests on the 8-GPU runner
ci:sglang SGLang integration tests: DeepSeek-R1-MXFP4 accuracy, Qwen 3.5 accuracy
ci:atom ATOM benchmark: DeepSeek-R1-0528, GPT-OSS-120B
ci:atom_full ATOM accuracy suite for PR and main models from ATOM models_accuracy.json
ci:vllm vLLM benchmark: GPT-OSS-120B, DeepSeek-R1-0528, Kimi-K2.5
ci:all All standard extended tests (excludes ci:atom_full)

Only add ci:atom_full for FlyDSL or Triton upgrades.
Add labels via the sidebar or gh pr edit 5451 --add-label <label>

PR title tags & labels:
Component tags ([Triton/Gluon], [HIP], [CK], [ASM], ...) are added to the PR title and as PR labels automatically from the changed files and re-synced on every push — change-type tags like [fix]/[Perf], op tags like [MLA], and human labels (ci:*) are left untouched. Add the no-auto-title label to opt this PR out.

Comment thread op_tests/test_glm52_routed_mxmoe.py Outdated

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.

should not add a completely new test only for a new instance. test could be added to existing ones possibly.

@pbkowalski pbkowalski Sep 11, 2026 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Removed the one-off test file and folded the routed-shape output_aux coverage into op_tests/test_moe_sorting.py.

Signed-off-by: Pawel Kowalski <pawel.kowalski@silo.ai>
@pbkowalski
pbkowalski force-pushed the pr/glm52-routed-mxmoe branch from cb9aaa8 to c7abb5c Compare September 11, 2026 13:40
@pbkowalski
pbkowalski marked this pull request as ready for review September 11, 2026 14:26
@pbkowalski
pbkowalski requested a review from a team September 11, 2026 14:26
@github-actions github-actions Bot changed the title [Feature] Add routed GLM-5.2 TP4 MXFP4 MoE shape [HIP] [Feature] Add routed GLM-5.2 TP4 MXFP4 MoE shape Sep 11, 2026
@github-actions github-actions Bot added the HIP label Sep 11, 2026
@zufayu
zufayu self-requested a review September 14, 2026 01:43
@zufayu

zufayu commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Advisory review (static + hand-run; not a merge gate). Validation/Perf ran no GPU stage — reasons are on their lines below. Findings tagged [verified] are traced to code/repro.

Adds the routed-only GLM-5.2 TP4 MoE shape (256 experts, top-k 8, H=6144, INTER=512) to the MXFP4 MoE aux-kernel codegen, five gfx950 tuned decode rows for it, and an output_aux sorting test.

Review (advisory): 🔴 HIGH RISK
Validation (deterministic): NOT RUN — REVIEW_AUTO_VALIDATE=0 (environment gap, a target existed: op_tests/test_moe_sorting.py); the PR is also STALE against merge target ba5f08e (all three hunks fail; #4526 rewrote this area after the PR opened), so a head-matched run is not well-defined until it is rebased.
Perf (advisory): NOT RUN — the PR is stale vs the merge target (no runnable head tree to measure base vs head back to back) and this box's GPU is gfx942 while the tuned rows target gfx950; the PR's +3.4%..+9.9% MI355X TP4 serving-throughput numbers are unreproduced here.

🔴 [verified] gen_instances.py adds (256,6144,512,8) to the codegen SHAPES, but on the current merge target the hand-maintained Python mirror MXFP4_MOE_SUPPORTED_SHAPES in aiter/ops/moe_mxfp4_aux.py — header comment "Keep synchronized with moe_aux/codegen/gen_instances.py::SHAPES", introduced by #4526 two days after this PR opened — does not contain the tuple (verified programmatically: is_mxfp4_moe_shape_supported(256,6144,512,8) returns False while the shared-expert (257,6144,512,9) returns True). A mechanical rebase that only resolves the conflicts leaves the new CSV rows dead config: for a routed GLM-5.2 TP4 decode call (expert=256, model_dim=6144, inter_dim=512, topk=8), aiter/fused_moe.py:3002 discards the tuned MXMOE config ("generated MXFP4 auxiliary kernels do not cover ...") and falls back to untuned default heuristics, and where a dedicated config file is passed, fused_moe.py:3040 then raises NotImplementedError — the PR's entire purpose (routed tuned decode) silently never takes effect. Author must rebase and add (256,6144,512,8) to MXFP4_MOE_SUPPORTED_SHAPES in aiter/ops/moe_mxfp4_aux.py alongside the gen_instances.py entry.

⚠️ [verified] The new test_moe_sorting_output_aux in op_tests/test_moe_sorting.py passes the 7-element output_aux tuple (sorted_ids, sorted_weights, sorted_expert_ids, num_valid_ids, moe_buf, m_indices, reverse_sorted — every output_aux return path, at the PR base and on main, returns 7) into _compare_moe_sorting_outputs, which unpacks exactly 5 names — ValueError: too many values to unpack (expected 5) fires before the first assertion, so the test cannot pass as shipped and validates nothing about the new shape; the PR body's documented repro command also passes -p t to the int-typed -p/--padding argument, which argparse rejects, so that command was not run verbatim. Author must pass out[:5] to the helper (or extend it to 7) and re-run the documented command exactly as written.

📝 [inferred] The routed rows cover only token buckets 2/4/16/32/64 while the existing shared-expert (257,9) rows in glm5_fp4_tuned_fmoe.csv cover all 16 buckets 1..32768 — a routed decode at token=1 or 8, or any prefill bucket, finds no row, and on the dedicated config-file path aiter/fused_moe.py:3040 raises NotImplementedError for a missing exact row (the token-tier fallback in _lookup_cfg applies only to the merged path and only above token 32768). Reviewer should ask which token buckets the routed call actually sees in serving, and why token=1 and 8 were left untuned.

📝 [verified] The new test in op_tests/test_moe_sorting.py runs at BLOCK_SIZE_M=32, which dispatches to the threestage instance aux_sort3s_NE256_TOPK8_MB32, while the new rows in glm5_fp4_tuned_fmoe.csv select block_m=16, which dispatches to the inline-quant zero-init instance aux_sortzi_NE256_TOPK8_MB16_H6144 (key construction in csrc/kernels/mxfp4_moe/moe_aux/moe_mxfp4_aux.cu) — the instance class the new config actually selects is exercised only by the separate tuning job (op_tests/tuning_tests/test_run_config.py::test_fmoe runs every tuned_fmoe row through gemm_moe_tune.py --run_config), not by the test added for this shape. Reviewer should ask whether a CI job that runs the tuning config test is guaranteed to execute on this PR, so the block-16 instance for (256,6144,·,8) is covered before merge.

📝 Two structural AI-code checks fired (the reused compare helper was not adapted to the new 7-element output; the PR's test claim does not match what the shipped code does, and the documented command is invalid as written) — elevated AI code risk on top of the stale rebase. Reviewer should ask for the actual run log of the documented test command and the serving sweep before approving the rebase.

@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

No activity for 15 days, so this is now labelled stale. A push or a comment clears it; keep-open exempts it. Nothing is closed automatically.

@github-actions github-actions Bot added the stale The PR hasn't been updated in 2 weeks label Oct 7, 2026

This branch has not been deployed

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

Labels

HIP stale The PR hasn't been updated in 2 weeks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants