Repository navigation
[AMD] fix: do not emit a shared-expert marker twice on the per-rank slot path - #36515
Merged
HaiShaw merged 2 commits intoAug 30, 2026
Merged
Conversation
…lot path On the per-rank fused shared-slot path the shared expert is appended after the gate by fused_append_remap_shared_experts_deepep, but select_experts also passes num_fused_shared_experts down to the gate, so both emit a marker. That costs a routed expert and corrupts an id. The gate is asked for K_routed = top_k - num_fused_shared_experts and then spends one of those slots on its own marker, so a top-6 routing silently becomes top-5. The marker it writes sits at id num_experts, which the DeepEP per-rank remap shifts one past the end of the expert space -- 384 -> 392 for 384 routed experts on EP8, where the valid ids are 0..391. Neither shows up as an error. The routing just carries one fewer expert and one out-of-range id per token, which is why this is worth fixing separately from the work that found it. Pass 0 to the gate when has_per_rank_fused_shared_slots() holds, leaving every other path exactly as before. Co-authored-by: Cursor <cursoragent@cursor.com>
karverma-amd
marked this pull request as ready for review
August 26, 2026 15:04
karverma-amd
requested review from
BBuf,
Edwardf0t1,
Fridge003,
HaiShaw,
Ying1123,
ch-wan,
ispobock and
merrymercy
as code owners
August 26, 2026 15:04
5 tasks done
Collaborator
|
/tag-and-rerun-ci |
HaiShaw
approved these changes
Aug 29, 2026
kediwu0331
pushed a commit
to Zhylkaaa/sglang
that referenced
this pull request
Aug 30, 2026
kevin-mii
added a commit
to kevin-mii/sglang
that referenced
this pull request
Sep 12, 2026
…aths On ROCm with aiter, `select_experts` asks the router for K_routed experts and `_post_process_topk_ids` appends the fused shared expert itself (fused_append_shared_experts, weight 1.0). For sigmoid+bias models the router is the Triton `moe_fused_gate`, whose `topk` is the total width including the shared slot; passing `num_fused_shared_experts` to it as well made each row [3 routed, shared, shared]: one routed expert lost and the shared expert counted twice. PR sgl-project#36515 fixed this for the DeepEP per-rank-slot path only. MiniMax-M3 MXFP8 TP4 on MI350X, GSM8K-500 (5-shot, 500 q): fusion off (aiter MoE) 0.884 fusion on, before 0.810 / 0.818 / 0.814 / 0.816 fusion on, after this change 0.896 Now the gate gets num_fused_shared_experts=0 whenever `_use_aiter`, so every aiter path appends the shared expert exactly once. Adds a GPU regression test that runs select_experts with `_use_aiter` patched on and checks the final row is the HF top-4 (renormalized, x routed_scaling_factor) plus one shared column of weight 1.0; it fails on the previous code. Repro/diagnostics used (benchmark host): /scratch/run/topk_chain_unit.py (prints the row before/after), /scratch/run/moe_fusion_unit.py (aiter FlyDSL per_1x32 kernel + sglang shuffle vs fp32 reference on real layer-3 weights: kernel was correct, best-fit scale 0.997), /scratch/run/topk_unit.py. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MATMHzpi6K7N2zkpo7mZLc (cherry picked from commit 5da5c8f)
kevin-mii
added a commit
to kevin-mii/sglang
that referenced
this pull request
Sep 14, 2026
…aths On ROCm with aiter, `select_experts` asks the router for K_routed experts and `_post_process_topk_ids` appends the fused shared expert itself (fused_append_shared_experts, weight 1.0). For sigmoid+bias models the router is the Triton `moe_fused_gate`, whose `topk` is the total width including the shared slot; passing `num_fused_shared_experts` to it as well made each row [3 routed, shared, shared]: one routed expert lost and the shared expert counted twice. PR sgl-project#36515 fixed this for the DeepEP per-rank-slot path only. MiniMax-M3 MXFP8 TP4 on MI350X, GSM8K-500 (5-shot, 500 q): fusion off (aiter MoE) 0.884 fusion on, before 0.810 / 0.818 / 0.814 / 0.816 fusion on, after this change 0.896 Now the gate gets num_fused_shared_experts=0 whenever `_use_aiter`, so every aiter path appends the shared expert exactly once. Adds a GPU regression test that runs select_experts with `_use_aiter` patched on and checks the final row is the HF top-4 (renormalized, x routed_scaling_factor) plus one shared column of weight 1.0; it fails on the previous code. Repro/diagnostics used (benchmark host): /scratch/run/topk_chain_unit.py (prints the row before/after), /scratch/run/moe_fusion_unit.py (aiter FlyDSL per_1x32 kernel + sglang shuffle vs fp32 reference on real layer-3 weights: kernel was correct, best-fit scale 0.997), /scratch/run/topk_unit.py. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MATMHzpi6K7N2zkpo7mZLc (cherry picked from commit 5da5c8f) (cherry picked from commit 587c277)
This was referenced Oct 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
On the per-rank fused shared-slot path the shared expert is appended after the
gate, by
fused_append_remap_shared_experts_deepep. Butselect_expertsalsopasses
num_fused_shared_expertsdown into the gate, so both of them emit ashared marker.
Two things go wrong, neither of which raises:
A routed expert is lost. The gate is asked for
K_routed = top_k - num_fused_shared_expertsslots and then spends one of themon its own marker, so a top-6 routing silently becomes top-5.
An id lands out of range. The gate's marker is written at id
num_experts,and the DeepEP per-rank remap then shifts it one past the end of the expert
space —
384 -> 392for 384 routed experts on EP8, where the valid ids are0..391.Modification
Pass
0to the gate whenhas_per_rank_fused_shared_slots()holds, so theappend is the single producer of the shared slot. Every other path keeps the
existing value, so this is a no-op off the per-rank shared-slot path.
14 lines in one file.
Accuracy
DeepSeek-V4-Pro, MI355X (gfx950), TP8 + DP8 attention + EP8/MoRI with
--enforce-shared-experts-fusion, which is the regime where the per-rankshared-slot path is live. The arms differ only by this fix; each server asserts
which side it is on before measuring.
GSM8K, 1319 questions, parallel 1319:
I would not read that gap as the effect. Four questions out of 1319 is inside
run-to-run noise for this benchmark, so the honest summary is that GSM8K cannot
resolve this bug either way — it is reported to show the fix does not regress,
not to claim it wins.
The case for the fix is the two defects themselves: a top-6 routing that is
actually running top-5, and an expert id written one past the end of the id
space. Both are silent, and neither is something a routing path should be doing
regardless of what an end-to-end score says.
Commands
Checklist
CI States
Latest PR Test (Base): ✅ Run #33272334686
Latest PR Test (Extra): ❌ Run #33272334475
Latest PR Test (AMD ROCm 7.2): ❌ Run #33272334891