swiglu_prefill: the padding comment has been stale since #76 - #190
Open
atassis wants to merge 2 commits into
Open
swiglu_prefill: the padding comment has been stale since #76#190atassis wants to merge 2 commits into
atassis wants to merge 2 commits into
Conversation
…they raise None of the three operators pad; each raises ValueError from __post_init__ on a non-divisible shape. seq_len_padded etc. just echoed gemm_1's unchanged M/K/N, so the name and comment both claimed behavior that stopped existing in amd#76 (2026-03-23) without the comment being corrected. Rename to *_aligned and describe why we read the dims off gemm_1 at all.
andrej
approved these changes
Sep 9, 2026
Co-authored-by: André Rösti <androsti@amd.com>
andrej
enabled auto-merge
September 9, 2026 15:48
Contributor
CI Test Results72ca7ac (2026_09_09_16_03_28) IRON - CI SummaryExamplesiron/applications/llama_3.2_1b
Smalliron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/mha
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Krackan - SmallIRONTested on iron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/mha
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Krackan - ExamplesIRONTested on iron/applications/llama_3.2_1b
Phoenix - SmallIRONTested on iron/operators/axpy
iron/operators/dequant
iron/operators/elementwise_add
iron/operators/elementwise_mul
iron/operators/gelu
iron/operators/gemm
iron/operators/gemv
iron/operators/layer_norm
iron/operators/leaky_relu
iron/operators/mem_copy
iron/operators/relu
iron/operators/repeat
iron/operators/rms_norm
iron/operators/rope
iron/operators/sigmoid
iron/operators/silu
iron/operators/softmax
iron/operators/strided_copy
iron/operators/swiglu_decode
iron/operators/swiglu_prefill
iron/operators/tanh
iron/operators/transpose
Phoenix - ExamplesIRONTested on Trend tables omitted, the comment hit GitHub's size limit. Full report in the workflow run. |
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.
SwiGLUPrefill.__init__says "All operators (GEMM, SiLU, ElementwiseMul) apply their own padding to meet hardware alignment requirements. We store the padded dimensions from GEMM" and then readsself.seq_len_padded = gemm_1.M. None of the three pad: each raisesValueErrorfrom its own__post_init__on a non-divisible shape (iron/operators/gemm/op.py,iron/common/operator_bases.py). Sogemm_1.Mis justseq_lenunchanged, andSwiGLUPrefill(seq_len=300, embedding_dim=2048, hidden_dim=2048)raisesValueError: M (300) must be a multiple of 256instead of padding to 512 as the comment implies.The comment was true when written (#50, 2025-12-11: GEMM really did pad then). Padding was removed in #76 (2026-03-23); #88 (2026-04-06) is the commit that made raise-only validation the norm and reworded these lines without correcting them, and #132 (2026-07-09) touched the same lines again, still describing padding. So this has been stale for about five months across two more edits, not a one-off typo.
This overlaps with #106 (open, unmerged), which hits the same root cause from the small-
seq_lenside by addingtile_m/tile_k/tile_noverrides somin_Mdrops below GEMM's default 256. That's a real fix for making small prefill batches reachable; this PR is narrower and just makes the comment and field names match what the code has actually done since #76 -- it doesn't touch tile overrides and shouldn't conflict with #106.Added
iron/tests/operators/swiglu_prefill_no_padding.py: construction-only tests (no device) showing a non-alignedseq_lenraises, and that the*_alignedfields equal the input dims rather than a padded value.Changed
iron/operators/swiglu_prefill/op.py: corrected the comment, and renamedseq_len_padded/embedding_dim_padded/hidden_dim_paddedto*_aligned(no external callers of the old names).Removed
Evidence
Confirmed against unmodified
devel(deb6e1e7) first:SwiGLUPrefill(seq_len=300, embedding_dim=2048, hidden_dim=2048)raisesValueError: M (300) must be a multiple of 256; atseq_len=512it constructs withseq_len_padded == 512, the unchanged input.PR Merge Checklist
develcommit and pointing todevel.