refactor: remove 836 lines of unreachable asm_templates code, and share the silu/gelu emitter - #73
Merged
Merged
Conversation
Three symbols in asm_templates/ have no call site anywhere in
PLENA_Compiler, PLENA_Simulator, PLENA_Tools, or PLENA_RTL:
- ffn_asm._ffn_asm_fused_up_gate (438 lines) plus the
`use_fused_up_gate` parameter and its dispatch branch, which
were the only way to reach it.
- ffn_asm.ffn_intermediate_asm (265 lines). 215 of its lines are
byte-identical to the live _ffn_asm_with_loops, so it was also
the single largest contributor to this file's 43% clone rate.
- gemv_asm.py (107 lines), which is additionally one of the five
copies of the weight_row/act_col projection emitter that have
silently diverged on whether the sub-column offset is
mlen-scaled. Deleting it removes one diverged copy for free.
ffn_asm.py drops from 1857 to 1131 lines.
Behavior is unchanged, verified by byte-comparison rather than by
inspection. Before and after this commit, on this same merge base:
codegen AICrossSim/clm-60m --seq-len 32 --num-layers 1
-> clm60m.asm sha256 a828a810... (78670 lines) identical
-> clm60m.mem sha256 37ec7f62... (78367 lines) identical
codegen HuggingFaceTB/SmolVLM2-256M-Video-Instruct (same flags)
-> smolvlm2.asm sha256 ee6fd302... (4515987 lines) identical
-> smolvlm2.mem sha256 ab182359... (4438689 lines) identical
pytest assembler/tests 4 passed -> 4 passed
pytest asm_templates/tests 31 passed -> 31 passed
pytest aten/tests 2 failed, 81 passed -> unchanged
pytest generator/tests 1 collection error -> unchanged
The aten/tests failures and the generator/tests collection error are
pre-existing on this merge base and unrelated: the former are a
missing router comment string and a quantization-accuracy threshold,
the latter is `No module named 'memory_mapping'` (PLENA_Tools is not
checked out in this environment).
An import sweep over every top-level package is unchanged except for
asm_templates.gemv_asm, which this commit removes.
`ffn_intermediate_asm` and `gemv_asm` were listed in
asm_templates.__all__. Nothing in any of the four repositories
imports either name, so removing them is safe, but it is technically
a reduction of the package's public surface.
The two docs tables that listed gemv_asm.py are updated in the same
commit so the removal does not create fresh documentation drift.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
SiLU(x) = x*sigmoid(x) and GELU(x) ~= x*sigmoid(1.702*x) were two
copies of the same emitter. They used the same three registers in the
same roles, the same num_vectors computation, and the same
negate / exp / +1 / reciprocal / multiply-in-place sequence inside the
same C_LOOP.
The emitted output differed in exactly four places, all of which the
new `pre_scale_fp_address` parameter absorbs:
- the banner comment,
- whether `S_LD_FP f2` is emitted,
- whether a leading `V_MUL_VF ..., f2` is emitted,
- whether the `V_SUB_VF` negation reads the scratchpad (GELU, which
negates the scaled value) or the activation (SiLU).
Both public functions keep their exact signatures and docstrings;
they are now thin wrappers over asm_templates/_sigmoid_activation.py.
Behavior is unchanged, verified two ways.
1. Differential sweep against the pre-refactor implementations taken
from origin/main: 3888 parameter combinations across register
triples, vlen, batch_size, hidden_dim, both FP constant addresses,
and activation/scratchpad base addresses chosen to straddle the
IMM2 boundary where _load_large_int changes shape. Zero mismatches.
2. End-to-end codegen on this merge base is byte-identical:
clm60m.asm/.mem sha256 a828a810... / 37ec7f62...
smolvlm2.asm/.mem sha256 ee6fd302... / ab182359...
and pytest asm_templates/tests stays at 31 passed.
Coverage note: the end-to-end goldens exercise GELU 12 times (in
SmolVLM2) but never reach SiLU, so SiLU's guarantee rests on the
differential sweep in (1) rather than on the goldens.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Behavior-preserving cleanup of
asm_templates/. No numeric, API, or emissionchanges — every claim below is backed by a byte-comparison, not by inspection.
Net: -951 / +96 lines,
ffn_asm.py1857 → 1131.Commit 1 — delete unreachable code (836 lines)
Three symbols have no call site anywhere in PLENA_Compiler, PLENA_Simulator,
PLENA_Tools, or PLENA_RTL:
ffn_asm._ffn_asm_fused_up_gateuse_fused_up_gateparameter and dispatch branch, the only way to reach itffn_asm.ffn_intermediate_asm_ffn_asm_with_loops— it was the single largest contributor to this file's 43% clone rategemv_asm.pyweight_row/act_colprojection emitter that have diverged on whether the sub-column offset is mlen-scaled; deleting it removes one diverged copy for freeCommit 2 — share one emitter between
silu_asmandgelu_asmThe two were the same emitter. Emitted output differed in exactly four places
(banner text, whether
S_LD_FP f2is emitted, whether a leadingV_MUL_VF ..., f2is emitted, and whether the
V_SUB_VFnegation reads the scratchpad or theactivation). One
pre_scale_fp_addressparameter absorbs all four. Both publicfunctions keep their exact signatures and docstrings.
Verification
Captured on this exact merge base, before and after:
clm60m.asm(78,670 lines)a828a810…clm60m.mem(78,367 lines)37ec7f62…smolvlm2.asm(4,515,987 lines)ee6fd302…smolvlm2.mem(4,438,689 lines)ab182359…Both models are the ones
ci.yml's codegen-smoke job builds, at the same flags.assembler/testsasm_templates/testsaten/testsgenerator/testsThe
aten/testsfailures (a missing router comment string, and aquantization-accuracy threshold at 89.8% vs >95%) and the
generator/testserror (
No module named 'memory_mapping'— PLENA_Tools is not checked out inthis environment) are pre-existing on
mainand untouched by this PR.For commit 2 specifically, a differential sweep ran the pre-refactor
implementations from
mainagainst the new ones over 3,888 parametercombinations (register triples × vlen × batch × hidden_dim × both FP constant
addresses × base addresses straddling the IMM2 boundary where
_load_large_intchanges shape). Zero mismatches.An import sweep over every top-level package is unchanged except for the
removal of
asm_templates.gemv_asmand the addition ofasm_templates._sigmoid_activation.Things worth a reviewer's attention
ffn_intermediate_asmandgemv_asmwere listed inasm_templates.__all__.Nothing in any of the four repositories imports either name, so removal is
safe, but it is technically a reduction of the package's public surface.
SiLU, so SiLU's guarantee rests on the differential sweep rather than on the
goldens.
gemv_asm.pyare updated in the same commit sothe removal does not create fresh documentation drift.
🤖 Generated with Claude Code