MoE: shared experts, arbitrary V_TOPK expert shapes, and an enforced stage-marker contract - #69
Merged
Merged
Conversation
This was referenced Jul 30, 2026
qichao-arlo-wang
force-pushed
the
feat/shared-expert-moe
branch
from
July 31, 2026 00:56
311561e to
60239b0
Compare
qichao-arlo-wang
force-pushed
the
feat/shared-expert-moe
branch
from
August 3, 2026 12:22
60239b0 to
5dbd87e
Compare
[Group A — ISA: routing at arbitrary expert shapes · 1 of 1] V_TOPK's rmask was a two-entry table: 0 = 32 experts/top-4 (GPT-OSS), 1 = 128/top-8 (Qwen3-30B-A3B). Every other production MoE shape falls outside it -- Qwen2-MoE 60/top-4, DeepSeek-V2-Lite 64/top-6, DeepSeek-V3 and Kimi K2 256/top-8, Llama-4 Scout 16/top-1 -- so each new architecture would have cost an ISA revision plus a matching emulator change. Add C_SET_TOPK_REG (6'h38), a sticky control register in the same family as C_SET_SCALE_REG / C_SET_STRIDE_REG / C_SET_V_MASK_REG, holding (num_experts << 8) | top_k. rmask=15 escapes to it; 0 and 1 keep their exact previous meaning. The 8-bit shift keeps the packed value inside a single 22-bit S_ADDI_INT immediate for every shape up to 16383 experts, so no S_LUI_INT pair is needed. It bounds top_k at 255, which no published MoE approaches. The register is a rd-only form, so parse_asm_file already handles it and only the encoder set needed the entry. The emulator half -- decoding the register and executing the rmask=15 escape -- is the matching Group A commit in PLENA_Simulator. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ontract Adds the shared-expert branch of MoE and replaces the undeclared ASM-comment contract that stage profiling depended on -- then makes that contract impossible to violate silently. Stage attribution used to be an *undeclared* cross-repo contract: the emulator substring-matched prose like "GPT-OSS gather token rows" out of the emitted comments. Rewording silently reclassified instructions, and there was no way at all to introduce a stage the substring table did not already know -- which is exactly what made shared experts unmeasurable. Emitters now emit `; @stage=<name>`, validated against MOE_STAGES at ASM-gen time so a typo fails immediately instead of collapsing a region into `other`. Markers are authoritative and sticky, so `stage` became a required parameter wherever one emitter serves several phases. A *default* there is a silent wrong answer rather than a missing one: the shared-expert sigmoid gate inherited `expert_route_weight` and misattributed instructions while every total still added up and every test stayed green. Dropping the defaults on moe_materialize_route_weights_for_active_rows_v0, moe_true_zero_vram_rows_v0 and moe_expert_activation_v0 is a breaking signature change; the simulator testbench call sites it turns into hard errors are fixed in the companion PR. A shared expert is an FFN every token passes through, summed into the routed output unweighted: y = shared(x) + sum_k route_weight_k * routed_expert_k(x) New emitters: moe_shared_expert_v0 (dense FFN over all rows with static weights), moe_shared_gate_v0 (Qwen2-MoE's sigmoid(x @ w_gate)), moe_combine_shared_and_routed_v0 (the unweighted add), and fused_shared_intermediate() for DeepSeek's pre-concatenated n_shared. SHARED_VS_ROUTED_NOTE is declared here as canonical text: the routed lowering is pair-major, so a shared-vs-routed cycle ratio reflects this compiler's lowering at least as much as the architecture, and that caveat has to travel with the measurement into the profile JSON. test_moe_stage_attribution.py enforces the required-`stage` rule and closes the routes that supply it without a signature default (lambdas, functools partial bindings, decorators). It also scopes the lint to MoE callees, so correct attention code like qkt_multiply(stage="decode") is not reported -- a lint that fires on correct code gets suppressed, and then protects nothing. `residual_setup` joins MOE_STAGES: MoE input preparation was being labelled `accumulator_init`, which is the combine accumulator that expert outputs are scattered into. The two qwen3_router_logits_* emitters gain the `router_topk` marker for the same reason -- both lower through general-purpose projection helpers that carry no marker, so the whole router GEMM was billed to whatever stage preceded the call. This also completes the gpt_oss_* -> moe_* API generalization; every old name survives as an alias, though the two whose `stage` became required now need that argument.
…nches dispatch [Group F — CI: the guard actually runs · 1 of 1] The lint of Group D landed with no workflow invoking it. `ci.yml` ran the generator parser tests and a codegen smoke, and nothing under `aten/tests` at all -- so the guard against a defaulted `stage` parameter could not have failed a pull request. A test nobody runs is indistinguishable from a test nobody wrote. Add a `moe-stage-guard` job. It needs neither torch nor a checkpoint, because the guard reads sources with `ast` rather than importing them; pyyaml is the one dependency, and only because collecting anything under `aten/tests` imports `aten/__init__.py`, which pulls the op registry in. That keeps the job at pytest + pyyaml and a couple of seconds, so it does not sit behind `needs: syntax-check`. Also add `test_every_test_file_here_is_wired_into_ci`, because naming a single file in a workflow step reintroduces the same hole one directory over: the next test file added here would again run nowhere. It asserts every `aten/tests/test_*.py` is either named in `ci.yml` or listed in `_UNWIRED_TESTS`. The five files on that list all import torch and some want a real checkpoint, so wiring them is separate work -- pinning them is what stops a new unwired file from hiding among them. The list is checked in both directions, so an entry that gets wired up or deleted has to leave it. `workflow_dispatch` is here for a different reason and is deliberately kept: `pull_request` is filtered to `branches: [main]`, so a branch stacked on a parent branch fires no jobs at all, and the guard added by this very file could not run on the pull request that introduces it. Manual dispatch is the smallest fix that makes such a branch verifiable before it retargets main. This branch now targets main directly, so it no longer needs it -- but the next stacked branch will. The rest of the CI story is in PLENA_Simulator, whose guard jobs are wired up alongside the guards themselves: there, the workflow's job list is asserted complete by one of the guards, so deferring the wiring would break the intervening commits. Verified: the guard passes at 4/4 on pytest + pyyaml with no torch present; dropping an unwired file into the directory fails it by name; and a stale `_UNWIRED_TESTS` entry fails it too. Addresses Agent 2 CRIT#1 and the review finding on CI triggering. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
qichao-arlo-wang
force-pushed
the
feat/shared-expert-moe
branch
from
August 3, 2026 13:18
5dbd87e to
73771f1
Compare
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.
What this PR does
Adds the shared-expert branch of MoE, generalizes
V_TOPKto arbitrary expertshapes, and replaces the undeclared ASM-comment contract that stage profiling
depended on — then makes that contract impossible to violate silently.
Companion to AICrossSim/PLENA_Simulator#97,
which carries the emulator half and the submodule pin. Neither is useful without
the other.
How to read the commits
The history is grouped by intent, not by the order the work happened in.
Each commit's body opens with its group and position. Reading top to bottom gives
the argument rather than the chronology: what the ISA needed, what the attribution
contract is, what shared experts are, how the contract is enforced, what enforcing
it turned up, and what runs it.
C_SET_TOPK_REG, so a new expert shape is not an ISA revision@stage=markers andMOE_STAGESreplace substring-matched prosestagebecomes required; a lint that covers every route around itresidual_setup, and the Qwen router GEMMs that had no markerGroups C and E depend on B; D depends on C; the simulator's pin ladder steps
through A→C→D→E→F in that order. That ordering is the reason the two PRs can be
read side by side at any commit rather than only at the tip.
Group A —
C_SET_TOPK_REG: routing at any expert shapeV_TOPK'srmaskwas a two-entry table:0= 32 experts/top-4,1= 128/top-8.That covers GPT-OSS and Qwen3-30B-A3B and nothing else — every other production
MoE falls outside it:
Each would have cost an ISA revision plus a matching emulator change. New opcode
C_SET_TOPK_REG(6'h38) is a sticky control register in the same family asC_SET_SCALE_REG/C_SET_STRIDE_REG/C_SET_V_MASK_REG, holding(num_experts << 8) | top_k.rmask=15escapes to it;0and1keep theirexact previous meaning, so existing programs emit byte-identical ASM.
The 8-bit shift is chosen so every shape up to 16383 experts packs into a single
22-bit
S_ADDI_INTimmediate — noS_LUI_INTpair. It boundstop_kat 255,which no published MoE approaches.
C_SET_TOPK_REGis anrd-only form, soparse_asm_filealready handles it andonly the encoder set needed an entry.
Group B — explicit
@stage=markersStage attribution was an undeclared cross-repo contract: the emulator
substring-matched prose like
"GPT-OSS gather token rows"out of emittedcomments. Rewording silently reclassified instructions, and there was no way at
all to introduce a stage the substring table did not already know — which is
exactly what made shared experts unmeasurable.
Emitters now emit
; @stage=<name>, validated againstMOE_STAGESat ASM-gentime so a typo fails immediately instead of collapsing a region into
other.Markers are authoritative and sticky, so
stagebecame a parameter whereverone emitter serves several phases:
moe_true_zero_vram_rows_v0zeroes combine accumulators, gather padding androute-weight tiles — the emulator previously guessed between them from the
preceding comment's stage, a stateful rule that could not express "this clear
belongs to the shared branch" at all.
moe_materialize_route_weights_for_active_rows_v0serves both route weights andthe shared gate. Without the parameter every gate instruction bills to
expert_route_weight, making a program with no routing appear to spend timecomputing route weights.
MOE_STAGESnames the threeshared_expert_*stages one commit before theemitters that use them arrive. The vocabulary is the contract, and the emulator
holds its
StageKindenum equal to it in both directions, so declaring astage and emitting it have to be one step or the guard fails from both sides.
This group also completes the
moe_*API migration: the simulator testbench hadalready been written against a generalized
moe_*API — three of its files calleight methods that did not exist in this repo at all, plus a
policy_nameparameter no method accepted. Every old name survives as an alias
(
_DEPRECATED_METHOD_ALIASES), so in-flight callers and the GPT-OSS bring-uptests that predate the rename keep working unchanged.
moe_router_select_v0isnew: it generalizes the top-k emitter to arbitrary
(num_experts, top_k)viaGroup A's escape, preferring the fixed
rmasktable where it applies.Group C — shared experts
A shared expert is an FFN every token passes through, summed into the routed
output unweighted:
moe_inter × n_shared== routedNew emitters:
moe_shared_expert_v0— dense FFN over all rows with static weightsmoe_shared_gate_v0— Qwen2-MoE'ssigmoid(x @ w_gate)moe_combine_shared_and_routed_v0— the unweighted addfused_shared_intermediate()— DeepSeekn_shared→ fused MLP widthn_shared_expertsis deliberately not an emitter parameter. DeepSeek storesits shared experts pre-concatenated and instantiates a single
DeepseekV2MLPof width
moe_intermediate_size × n_shared_experts. That is a checkpoint-loadingconcern, not an emission one.
The fusion identity, stated precisely. Because SwiGLU is elementwise along the
intermediate axis while the down projection sums over it, the fused MLP equals the
sum of the individual shared experts in exact real arithmetic — an algebraic
identity, not an approximation. As emitted it is not bit-identical, for two
reasons unrelated to the algebra:
summing
n_sharedseparate reductions, and FP addition is not associative;so when
moe_intermediate_sizeis not a multiple of the block, the fusedtensor's blocks straddle expert boundaries and pick up scales derived from two
different experts — the quantised weights themselves differ, not merely their sum.
Neither makes the fusion wrong. They are stated because #97's shared-expert test
is bit-exact at
atol=rtol=0, so a test written against "exactly equal" wouldfail for entirely correct reasons and send someone hunting a bug that is not there.
SHARED_VS_ROUTED_NOTEis declared here as canonical text. The warning that ashared-vs-routed cycle ratio measures this compiler's lowering rather than MoE
hardware previously existed as prose in this module and, in different words, as
a hardcoded string in the emulator that ships it in the profile JSON — with
nothing checking they agreed. The wording of a caveat is the whole of its content.
The emulator keeps its own copy (it writes that JSON with no Python available) and
a test on that side asserts byte equality against this constant.
The gate reuses
moe_materialize_route_weights_for_active_rows_v0: one FP scalarper token broadcast across hidden is structurally identical to a
V_TOPKrouteweight, only the scalar's origin differs. The sigmoid itself runs in the scalar FP
unit (
S_SUB_FP/S_EXP_FP/S_ADD_FP/S_RECI_FP) — there is one value pertoken, and the vector form would compute MLEN copies of it.
Group D —
stagebecomes required, and stays requiredGroup B gave the reused emitters a
stageparameter. This group makes itimpossible to not supply one.
Markers are sticky, so an emitter called from inside a marked region inherits the
enclosing marker: a default for that parameter is a silent wrong answer rather
than a missing one. This is not hypothetical — the shared-expert sigmoid gate
inherited
expert_route_weightand misattributed 999 instructions while everytotal still added up and every test stayed green.
The default is dropped from
moe_materialize_route_weights_for_active_rows_v0,moe_true_zero_vram_rows_v0andmoe_expert_activation_v0. This is a breakingsignature change; the six simulator testbench call sites it turns into hard
errors are fixed in #97, in the commit that moves the pin onto it.
test_moe_stage_attribution.pyenforces the rule, and the four commits after itare each about a way the lint was not yet doing its job:
ast.FunctionDef, andast.AsyncFunctionDefis a sibling class, not a subclass, soasync defemitters were outside it entirelystageis not a reserved word;qkt_multiply(stage="decode")is correct attention code that the unscoped lint reported as an unknown MoE stage. A lint that fires on correct code gets suppressed, and then protects nothing_moe_stage_argumentsinspectednode.keywordsonly, and all 19moe_stage_marker(...)call sites pass the stage positionally — the lint was scanning three keyword arguments and none of the markers. Coverage went from 3 stage names to 25functools.partial/partialmethodbindings and decorators handedstage=each supply the argument without any call site naming itThe third row is the one worth pausing on: for several commits this branch shipped
a lint that provably could not catch the defect it was written for.
A decorator that injects a stage without naming it in its own call cannot be
detected without resolving what the decorator does. That limit is stated at the
helper, with a TODO saying the answer is an explicit allowlist rather than deeper
analysis.
Group E — attribution corrections found by review
Two holes on the same stretch of the decoder MoE sublayer. Neither was findable
before Groups B and D: the lint checks that a stage name is declared, not that
the region it labels is the region it describes.
residual_setupjoinsMOE_STAGES. MoE input preparation — the residualbuffer zero, the residual copy, the input RMSNorm and its norm-weight multiply —
was being labelled
accumulator_init, which is the combine accumulator thatexpert outputs are scattered into. Different thing, and the cost scales with
rows × hiddenrather than with the accumulator.qwen3_router_logits_*emitters gain therouter_topkmarker. Theyhad none, and both lower through general-purpose projection helpers that have
none either, so the entire router GEMM was billed to whatever stage preceded the
call.
moe_router_logits_bf16_v0already marked itself; these now match. This ispart of the same fix rather than a separate one: relabelling the residual zero
without it would have moved the Qwen router GEMM into
residual_setup— a newwrong attribution instead of the old one.
Markers are comments (
IsaBuilder().comment(...)), so no instruction count, cycletotal or numerical result changes — only which stage the profile bills.
Group F — CI
Before this,
ci.ymlran the generator tests and a codegen smoke and nothingunder
aten/testsat all — so the guard could not have failed a pull request,which is indistinguishable from not having written it. The
moe-stage-guardjobneeds neither torch nor a checkpoint (the lint parses source with
ast), so it ispytest + pyyaml and a couple of seconds.
Naming one file in a workflow step recreates the same hole one file over, so
test_every_test_file_here_is_wired_into_ciasserts everyaten/tests/test_*.pyis either named in
ci.ymlor listed in_UNWIRED_TESTS. The five files on thatlist all import torch; pinning them is what stops a new unwired file hiding among
them, and the list is checked in both directions.
workflow_dispatchis kept for a different reason:pull_requestis filtered tobranches: [main], so a branch stacked on a parent branch fires no jobs at all.This branch targets main directly and no longer needs it — the next stacked branch
will.
Validation
pytest aten/tests/test_moe_stage_attribution.py— 7/7, on pytest + pyyaml with notorch installed. Verified per commit across the branch: 0 syntax failures at all
12, and the guard passes at every commit where it exists (3 → 4 → 6 → 7 tests as
each gap closes).
Each guard was also verified to fail correctly: an injected defaulted-stage
lambda, a
partialbinding and apartialmethodbinding are each reported withfile and line; a
stage="acumulator_init"typo on a MoE callee is reported whileqkt_multiply(stage="decode")is not; an injectedmoe_stage_marker("expert_bais", ...)is reported at
program_routed_moe.py:921(and was not reported before thepositional-matching commit); an unwired test file and a stale
_UNWIRED_TESTSentry each fail by name.
From the simulator side (see #97): three shared-expert configurations bit-exact at
atol=rtol=0; six routing shapes end to end, including DeepSeek-V3's 256 expertsacross four MLEN-wide logit blocks selecting expert 255; all six pre-existing
routed-MoE CI tests still pass, including
gpt_oss_topk_testwhich now exercisesthe alias path; 87 emulator unit tests.
Known limitation, not introduced here
The routed path is pair-major: one
(token, expert)pair at a time in aBLEN-row slot, re-fetching expert weights per pair. Measured, its matrix-op count
is strictly linear in
tokens × top_kand matrix-engine row utilisation is fixedat 1/64 regardless of token count. The shared branch added here is properly
batched (3
M_MMfor 1 token or 64).So a shared-vs-routed cycle ratio measured today reflects the routed lowering's
per-pair overhead at least as much as the architecture. That is what
SHARED_VS_ROUTED_NOTEsays, and #97 now emits it asclassification.attribution_notes.shared_vs_routedso the caveat reaches thepeople reading the numbers rather than only the people reading this source.
Expert-major batching is follow-up work.
Boundary with Simulator #97
means re-lowering the routed path to token-major / expert-batched, which is
entirely a compiler concern. #97 explicitly does not touch it.
moe_shared_gate_v0,for token_idx in range(rows)) is a shared-path codegen follow-up tracked onthe #97 side, not part of the pair-major re-lowering. Static instruction count
there grows with batch size; it is unrelated to routed placement.
MOE_STAGESis a cross-repo contract, and it is now a checked one. #97'semulator-side guard parses this declaration out of the pinned submodule with
Python's own
astand compares it againstStageKindin both directions. Itused to compare against a hand-copied 13-entry array — a copy only fails when
somebody remembers to update it, which is the same failure mode as having no
guard, and the compiler side is the side that moves. Adding
residual_setuphere would not have failed the old test.
emitters, the
MOE_STAGESvocabulary and the lint overaten/plena; #97 ownsthe emulator's stage profile, the testbenches, the timing harnesses, and the
same lint pointed at
testbench/across the repo boundary. The submodule pin isthe only coupling, and it moves in that repository.
Note on branch naming
The branch is still called
feat/shared-expert-moe, which now under-describes it —it carries the ISA change, the attribution contract and its enforcement too.
Renaming would close this PR, so the name stays and this paragraph exists instead.
🤖 Generated with Claude Code