[Frontend] Build every tile descriptor from axes#298
Open
YWHyuk wants to merge 3 commits into
Open
Conversation
d9131ba to
1ff2ebb
Compare
…ach space A tile descriptor is stored column-wise today: one parallel array per space (the extents, the DRAM strides, the SRAM strides), a scalar index into them (vlane_split_axis), and a scalar riding alongside (vlane_stride). Keeping the columns aligned across an axis reorder, insert or collapse is the caller's job, and that is where the mistakes are. The reduction GEMM repeats the same "if nr_rdim" branch over four of them. apply_divisor inserts an axis but forgets tile_constraint. decompose_transfer re-indexes three arrays by hand and remaps the lane index separately. Axis stores the same table row-wise. One iteration dimension carries its extent, the stride of the DRAM access it walks, and the enclosing loop variable that advances it. What is not a property of a single axis stays on the tile: which axis rides the lanes, and the order the axes sit in SRAM. Two orders matter and they can differ. The axes' declared order is the memref's dimension order, which is what linalg sees. sram_order is the order they sit in SRAM, outermost first. The GEMM reduction variant declares its output (N, M) while still laying M out contiguously; today that is four separate branches over the extents, the SRAM strides, the lane axis and the index expression. The DRAM stride is the stride of the access, not of the tensor. Conv walks a padded, permuted layout, so it is a sympy expression, not a layout stride. build_tile() derives from that what the templates compute by hand: the SRAM strides, the lane axis, the lane stride, and the DRAM index expression. An extent-1 axis is indexed only at 0, so its SRAM stride never reaches an address: Spike bounds that axis' loop by its extent and multiplies the stride by the index (torchsim_mvin_common.h), and each stride is rescaled against the lane axis' one independently, so it does not perturb the others. Deriving it from the SRAM order gives the product it would have if it were not degenerate, which is what most templates already write by hand. No caller yet.
Each operand took eight lines: the tile extents, the SRAM strides worked out by hand as products of those extents, a constructor whose tile_size argument the next line overwrites, the lane axis as a bare integer, the buffer name as a string, and an index expression built from a list of loop symbols. Half of that is derived from the rest. State the decisions -- which extents, which DRAM stride each axis walks, which loop variable advances it, what order the axes sit in SRAM, which one rides the lanes -- and derive the rest. The SRAM strides are no longer typed out, so they cannot drift from the extents. The lane axis is named rather than positional. vlane_stride is 1 in every template, so it becomes a default nobody writes. The GEMM and BMM reduction variants declare their output tile transposed, (N, M) instead of (M, N), while laying M out contiguously either way. That used to be four separate "if nr_rdim" branches over the extents, the SRAM strides, the lane axis and the index expression, which could disagree. Now the declaration order flips and the SRAM order stays (n, m). This also kills a copy-paste that survived because the API was redundant: five templates built W_tile_desc with X_tile_size, harmless only because the next line overwrote it. conv_mt walks one kernel column at a time, so its k_w axis is degenerate. That was a bare 1 in a tile-size list; it is now an axis that says so. Verified by regenerating every kernel from scratch. The emitted MLIR is byte-identical for mm, mm+relu, addmm, mm+reduction, prologue-fused mm, the N==1 edge, and all four conv variants (single-batch, single-batch-strided, multi-tile, batched). BMM's degenerate batch axis picks up the SRAM stride it would have if it were not degenerate, so its transfers differ in that one entry; compiling both ways gives a byte-identical instruction body and differs in one slot of the DMA descriptor global. Functional mode matches CPU for all fourteen (max abs diff 4.2e-05), and tests/ops/fusion still fuses the same kernels.
…m axes The last four templates. No template constructs an MLIRMultiDimTile any more. sdpa's key tile is laid out transposed because key is the stationary operand of the systolic array. That was a stride list and a lane index sitting apart; it is now one sram_order and one lane name, with the reason next to them. sort's lane axis can have extent 1, since a one-dimensional sort runs in a single lane. cat names its tiles after the input rather than "<name>_buffer". maxpool, cat and sort emit byte-identical MLIR. sdpa's degenerate batch axis picks up the SRAM stride it would have if it were not degenerate, so four tile_stride entries change, the same way BMM's do. All four match CPU in functional mode: max_pool2d, cat, sort and topk are exact, and tests/ops/attention/test_sdpa.py passes for every head and sequence length it covers. GEMM, BMM and conv keep the kernels they had, and tests/ops/fusion still fuses the same ones.
a3bf0ed to
90d8b22
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.
A pure refactor of how the ten MLIR templates describe their tiles. The emitted MLIR is byte-identical everywhere except one entry of one attribute, explained and justified below. No template constructs an
MLIRMultiDimTileany more.The problem
A tile descriptor is stored column-wise: one parallel array per space, plus a scalar index into them and a scalar riding alongside.
Keeping those columns aligned across an axis reorder, insert or collapse is the caller's job, and that is where the mistakes are:
if nr_rdimbranch over four columns -- the extents, the SRAM strides, the lane axis and the index expression -- which can disagree;apply_divisor(mode="split")inserts an axis into_tile_sizeandtile_axis_order, bumpsvlane_split_axis, and forgetstile_constraint(dormant: it has no callers);decompose_transferre-indexes three arrays by hand and remaps the lane index separately;W_tile_descwithX_tile_size, harmless only because the next line overwrote it.The change
Store the same table row-wise. One iteration dimension carries what it means in each space it is embedded in.
What is not a property of a single axis stays on the tile: which axis rides the lanes, and the order the axes sit in SRAM.
build_tile()derives the SRAM strides, the lane axis, the lane stride and the DRAM index expression. The strides are no longer typed out, so they cannot drift from the extents. The lane axis is named rather than positional.vlane_strideis 1 in every template, so it becomes a default nobody writes -- the word disappears from all ten.Two orders, and they can differ
The axes' declared order is the memref's dimension order, which is what
linalgsees.sram_orderis the order they sit in SRAM. The GEMM and BMM reduction variants declare their output transposed,(N, M)instead of(M, N), while laying M out contiguously either way -- the bytes do not move, only the declaration. That was four separate branches; it is now one:The file
tile_axis.pyis 83 lines. Every template shrinks.The DRAM stride is not the tensor's stride
Conv walks a padded, permuted logical layout, so its per-axis DRAM stride is a sympy expression, not
layout.stride[dim].Axiscarries the expression directly.Facts the refactor surfaced
conv_mtwalks one kernel column at a time, so itsk_waxis is degenerate. That was a bare1in a tile-size list; it is now an axis that says so.sram_orderand onelane, with the reason next to them.The one attribute that changes
The templates disagreed on the SRAM stride of an extent-1 axis. conv writes the product it would have if it were not degenerate; BMM and sdpa write
0. Deriving it fromsram_ordergives conv's answer, so four BMMtile_strideentries and four sdpa ones change from0to that product.It never reaches an address. Spike (
torchsim_mvin_common.h) bounds that axis' loop by its extent and multiplies its stride by the index:For a non-lane axis of extent 1 the loop runs once with the index at 0. Each
block_stride[i]is rescaled against the lane axis' one independently, so it does not perturb the others;buffer_sizeanddma_buffer_stridecome fromdim_size, not from the strides.Compiling BMM both ways confirms it: the LLVM instruction body is byte-identical, and the two differ in exactly one slot of the
dma_descglobal -- the SRAM stride of a dimension whose extent is 1.Verification
Every kernel was regenerated from scratch (kernel directories deleted first, so the content-hash path could not mask a change).
tile_strideentry abovetests/ops/attention/test_sdpa.pytests/ops/fusion/(7 tests)What this does not do
MLIRMultiDimTileis unchanged;build_tile()is an adapter over it. The parallel arrays, thevmapscalars, thetile_axis_orderfloat ranks,apply_divisor's missingtile_constraintupdate,decompose_transfer's hand re-indexing andcompute_tile_size's eight copies ofvmap.vlane_strideall survive.That is deliberate. The callers had to stop passing derived values before the internals could change, and the byte-identity above is only meaningful because nothing else moved. The internals are the next step.