Repository navigation
[HIP] [CK] [FlyDSL] [Kernel] Extend MXFP4 GEMM1 replacement to A4W4 - #4526
Conversation
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
🏷️ CI GuideRuns automatically on every PR:
Extended tests (opt-in via labels):
|
There was a problem hiding this comment.
🟡 Human review recommended
It changes Tier-2 MoE dispatch (aiter/fused_moe.py) and multiple FlyDSL kernel/tuning/AOT paths, so the blast radius is high despite local unit test coverage.
Pull request overview
Extends the FlyDSL MXFP4 “GEMM1 replacement” MoE pipeline beyond the prior A4W4-only port to also cover A8W4, adding cache-safe kernel naming/selection, runtime gating, AOT precompile support, and updated tuned model CSVs.
Changes:
- Add cache-safe GEMM1 kernel-name construction/parsing for new MXMOE variants (FP8 intermediates, SiTUv2 beta encoding, interleaved layouts).
- Update tuning + dispatch plumbing to couple new GEMM1 variants with mixed_moe_gemm_2stage GEMM2 kernels, plus profiling support to split stage timings.
- Expand supported shape/config coverage (aux-kernel shape set, AOT job enumeration, refreshed tuned CSVs for GLM-5 / DeepSeek-V4 / Kimi-K3).
File summaries
| File | Description |
|---|---|
| csrc/kernels/mxfp4_moe/moe_aux/codegen/gen_instances.py | Extends generated aux-kernel shape list (adds GLM-5/Kimi-K3 shapes) and minor formatting tweaks. |
| csrc/ck_gemm_moe_2stages_codegen/tune_mxfp4_flydsl.py | New CLI entrypoint for the updated FlyDSL MXFP4 GEMM1 + mixed GEMM2 coupled tuner. |
| csrc/ck_gemm_moe_2stages_codegen/gemm_moe_tune.py | Updates tuning logic to generate/lock candidates for A4W4 and A8W4 (FP8) replacement paths and record stage timings. |
| aiter/test_common.py | Adds optional kernel-time extraction from trace profiling for more detailed perf attribution. |
| aiter/ops/moe_mxfp4_aux.py | Adds an explicit supported-shape set + helper for MXFP4 aux-kernel coverage checks. |
| aiter/ops/flydsl/mxfp4_kname.py | Implements new cache-safe GEMM1 naming/parsing (including float-bit encoding for SiTUv2 betas) and selection helpers. |
| aiter/ops/flydsl/mxfp4_gemm1_kernels.py | Extends GEMM1 port wrapper to support dtype/activation variants, BN choices, and SiTUv2 parameters. |
| aiter/ops/flydsl/moe_kernels.py | Threads model-dim padding into reduction compilation and normalizes padded output in debug/raw-output mode. |
| aiter/ops/flydsl/kernels/mxfp4_gemm_common.py | Adds FP8/FP4 MMA atom/scaling helpers and activation math helpers used by new variants. |
| aiter/ops/flydsl/kernels/moe_gemm_2stage.py | Adds model-dim padding support to the reduction kernel path and updates comments/metadata. |
| aiter/ops/flydsl/kernels/mixed_moe_gemm_2stage.py | Adds padding tags to module naming and bounds padded stores in stage2 epilog. |
| aiter/fused_moe.py | Extends runtime dispatch/gating to select MXMOE replacement kernels for both A4W4 and A8W4, including SiTUv2 beta threading and stage2 integration. |
| aiter/configs/model_configs/kimik3_a8w4_tuned_fmoe.csv | Updates tuned Kimi-K3 A8W4 configs to use new MXMOE GEMM1 kernel names. |
| aiter/configs/model_configs/glm5_fp4_newg1_mixedg2_stdsort_tuned_fmoe.csv | Adds a new tuned CSV for GLM-5 FP4 with new GEMM1 + mixed GEMM2 naming. |
| aiter/configs/model_configs/dsv4_fp8fp4_tuned_fmoe.csv | Updates DeepSeek-V4 FP8FP4 tuned rows to use MXMOE GEMM1 replacement kernels where intended. |
| aiter/aot/flydsl/mxfp4_moe.py | Expands AOT job parsing to cover fp4/fp8fp4/a8w4 tuned CSVs and generate correct compile jobs for replacement kernels. |
Review details
Suppressed comments (1)
aiter/ops/moe_mxfp4_aux.py:19
- The shape glossary line uses
? {16, 32, 64, 128}, which is unclear. Use a clear set-membership phrasing (e.g., "in {..}") so readers don't interpret it as uncertainty.
# MB = block_m (sort/gemm block size, ? {16, 32, 64, 128})
- Files reviewed: 17/17 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…526-20260803 Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # aiter/fused_moe.py # aiter/ops/flydsl/kernels/mixed_moe_gemm_2stage.py
Preserve the replacement and padding fixes on the MXMoE v2 APIs while applying CI formatting and review cleanups. Co-authored-by: Cursor <cursoragent@cursor.com>
Match the version used by the repository's Black action so the style check remains stable through local hooks. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Use the tuple form so the fused MoE dispatch passes PIE810 in the pinned CI Ruff version. Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: fsx950223 <fsx950223@outlook.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Preserve the sorted MXFP4 intermediate path across the FlyDSL MoE refactors and carry padded reduction handling into the extracted kernel. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🟡 Changes recommended
The updated FlyDSL topk-decode/test gating can raise import-time failures when FlyDSL is unavailable, and the new --output-aux help text is now inconsistent with the runtime BM16 selection logic.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
op_tests/test_topk_per_row.py:512
flydsl_availableis now gated only on arch. Ifflydslisn't installed/usable, theflydsl=Truedecode runs will fail at import time instead of being skipped.
flydsl_available = get_gfx() in ("gfx942", "gfx950")
op_tests/test_moe_2stage.py:784
- The
--output-auxhelp text says "block_m=16 keeps its fused sort regardless", butfused_moe._aux_uses_opus()can select Opus for block_m=16 when routed_rows >= num_experts. This makes the CLI help misleading for BM16 cases.
help="For configs whose sort emits the a4w4 aux arrays, pick which prologue "
"does it: 'opus' (the default those configs already select) runs the Opus "
"multi-phase sorter, one CTA per expert; 'threestage' forces the port's own "
"3-stage sort instead. Same sorted output either way, and block_m=16 keeps "
"its fused sort regardless. Passed straight through to "
"fused_moe(output_aux=...); configs that do not use the aux sort are "
"unaffected.",
- Files reviewed: 24/24 changed files
- Comments generated: 1
- Review effort level: Lite
All six reproduce; each is fixed at the site the reviewer identified. 1. inter_sorted_shuffled_scale under-allocated for BM16 native-scale shapes (fused_moe.py). The native layout addresses one padded chunk per M block -- out_as_per_chunk_dw_for rounds scale-N up to a multiple of 8 columns -- but the allocation sized on the unpadded D_INTER//32. For D_INTER=1408 that is 44 vs 48 columns: 45056 bytes allocated against a 49152-byte span, matching the ~4 KB of guard bytes the reviewer saw overwritten. Now takes the max with the padded stride. Checked across shapes: 1408 and 1152 were overflowing and are now covered; 384/512/896/1024/1536/2048 are unchanged. 2. The activation catch-all returned "silu" for anything unrecognised, so Gelu and GeluTanh -- real ActivationType values -- matched a SiLU-tuned row and ran silently. Replaced with get_flydsl_activation_name, which refuses, and the refusal now discards the config like the other rejections. The tuner's _row_act had the identical fold and would have validated a Gelu row against a SiLU reference, so it raises too. 3. A gate_mode mismatch warned and kept the tuned kernelName1 while flipping interleave. Those are not independent -- BN64 and k_wave>1 GEMM1 variants are compiled separated-only -- so it produced metadata that dies in _assert_supported. Tuned configs are now discarded and fall back to heuristics; an explicitly supplied mismatched name raises where the cause is still legible instead of failing later. 4. MXMOE still ignored a finite swiglu_limit for SiLU: _activation_mul_batch consumes it for swiglu only, while the public contract and the torch reference clamp. Rejecting the config lets the call fall back to a backend that does clamp, rather than returning quietly different numerics. get_2stage_cfgs takes swiglu_limit for this; both call sites forward it. 5. model_dim_pad was inserted ahead of existing optional parameters, so test_flydsl_moe_a8w4.py's seven positional args rebound to model_dim_pad=expert_mask and tripped the mutual-exclusion check. Caller switched to keywords, and the optional parameters are now keyword-only so a future insertion cannot rebind silently. CI never caught this because both collectors use find -maxdepth 1 and op_tests/flydsl_tests is not collected at all. 6. The D_HIDDEN check accepted a range the B-scale addressing does not cover: issue_b_scale_load selects between two bases 16 K tiles apart, so the address is only correct while K_C < 32 and from 32 aliases onto tile K_C-16 -- inside the buffer, so silently wrong. Added the fence. It rejects nothing shipped, though qwen3_8_2400b at model_dim 8192 sits exactly on the 32-tile limit, so the next size up would have hit it. Predates this PR: the B-scale addressing (6) and the activation fold (2). Fixed here because this PR defines the accepted range and the guard that consumes it. Signed-off-by: fsx950223 <fsx950223@outlook.com>
a46b91c made _run_moe_reduction's optional parameters keyword-only but only checked callers with a single-line grep, which cannot see a call split across lines. moe_kernels.py:2281 passes model_dim_pad, expert_mask and topk_ids positionally -- eight arguments in total -- so it would have raised TypeError on every reduce-mode stage2 with accumulate off. Converted it to keywords. Re-verified by walking the AST of every call site in the tree against the parsed signature rather than grepping: all six now pass five positional arguments and only known keywords. Signed-off-by: fsx950223 <fsx950223@outlook.com>
The review offered two ways to stop a future insertion silently rebinding arguments -- append the new optional parameter, or make the tail keyword-only. a46b91c took the second; this switches to the first, which is the lower-risk half of that choice. Keyword-only fixed the hazard but narrowed the calling convention: positional arguments beyond model_dim were rejected outright, and it broke moe_kernels.py:2281, which passes eight positionally. Appending model_dim_pad instead leaves every parameter above it on the position it occupies on main, so the pre-PR positional form binds exactly as it used to and no caller -- in tree or out of it -- can be rebound by this PR. No AOT change is needed: aiter/aot/flydsl/moe.py and mxfp4_moe.py never pass model_dim_pad at all, and both already call with five positional arguments plus keywords. Verified by walking the AST of every call site against the parsed signature: all six bind cleanly, and a simulated pre-PR positional call still lands on expert_mask/topk_ids with model_dim_pad defaulted. Signed-off-by: fsx950223 <fsx950223@outlook.com>
Correct interleaved weight and scale addressing for BN64 and K-wave specializations, and allow these layouts through compile/runtime validation. Keep GEMM1 config names aligned with main: select layout from gate_mode and emit separate AOT jobs for interleaved and separated variants.
Integrate the low-token MoE capability checks and bounded quantization launches with the activation-keyed GEMM1 variants and adaptive Opus sorting. Preserve main GEMM2 selections and migrate the new Kimi-K3 SiTUv2 rows to activation-specific kernel names. Validation: Black and Ruff passed for all 19 PR Python files; Python syntax and whitespace checks passed; 31 CPU config tests passed with GPU discovery stubbed; AOT/cache/dispatch and model CSV invariants checked. Local GPU validation is blocked by pre-existing ROCm driver hangs.
Apply output_aux backend overrides only when the selected metadata already emits auxiliary route data. Unsupported hybrid configurations retain their normal sorting and GEMM pipeline.
Select Opus auxiliary sorting exclusively from compatible tuned metadata and remove the public and CLI backend override.
Prune statically supported GEMM1 candidates by estimated expert M, with a full-search override shared by serial and multi-GPU tuning. Reject BM families without explicit pruning rules and report how to maintain them. Include process IDs and shape fields in pruning summaries. Add CPU regression tests for unknown BM errors, supported-family coverage, and the BM128 boundary. Document pruning thresholds, full-search retuning requirements, and same-BM pipeline pairing in the tuner README. Signed-off-by: Wulley <bernard.liu@amd.com> (cherry picked from commit 8d9892bb133cae20c828eb8aeed7c5ce3cc6b2be)
# Conflicts: # aiter/aot/flydsl/moe.py
Restore aiter/test_common.py to current main. The optional per-kernel profiler timing API now lives on split-pr4526-test-common.
Summary
_hpf), and expanded tuner coverage for BN / wave / K-wave / XCD choices.GEMM2 scope
GEMM2 tuning is intentionally out of scope for this PR.
The final config set was audited against latest main
e583a77c6:glm5_fp4_tuned_fmoe.csvkimik2_fp4_tuned_fmoe.csvkimik3_a4w4_tuned_fmoe.csvqwen3_5_397b_fp4_tuned_fmoe.csvFor all 81 audited rows,
kernelName2,us2, anderr2are byte-equivalent to latest main. Thirteen rows had also changedblock_m; those were retuned with main's GEMM2 andblock_mlocked. Rows without a confirmed GEMM1 gain fell back to the main GEMM1. The Qwen3.5-397B FP4 config is fully restored to main.Performance validation
Measured on one gfx950 MI355X system with warm FlyDSL caches. Main and PR were alternated on the same GPU; the 81-row set received three A/B rounds and every negative/borderline row received five-round confirmation.
Environment for Kimi-K3 A4W4:
Final result versus latest main:
logits_diff: 0.00733 (gate:<= 0.01)The two small-token Kimi-K2 rows that regressed with the unchanged kernel were recovered with HPF GEMM1 variants:
inter_dim=512: +0.82%inter_dim=256: +1.47%Validation
Test methodology
Config comparisons run the production
fused_moepath. Main and PR use separate checkouts and caches but execute on the same GPU in alternating order. A row is retained only when repeated measurements show a real gain; otherwise its main config is preserved. Per-stage attribution usesrocprofv3 --kernel-trace, while final acceptance is based on end-to-endfused_moelatency andlogits_diff <= 0.01.