Repository navigation
driver: blocked outer traversal, enabled where it is measured to pay - #101
Merged
Merged
Conversation
… unused) The consolidated design for matching the reference on transposed-output contractions is `docs/design/blocked-outer-traversal.md`; it records the measured problem (pack_a 27.6 ms of 43.1 on `abjc-cbka-kj` f64 16 MiB 1T), the four pre-reviews' findings, and the plan. This commit is the first step of that plan: the block membership, its conservative eligibility predicate, and the gather that turns global scatters into block-local ones. `driver/block.rs`: - `eligible` returns a block shape only when the role has two or more live axes and the operand's fastest axis is *not* the output's - i.e. when today's consecutive slices of one flattened scatter cannot hold the runs the design needs. Every other shape keeps today's path by construction. - `rows` enumerates a block's global flattened logical row indices with the operand's fastest axis innermost, which is the order the panel rows take, and which is why the pack needs no contract change. - `gather` turns a global scatter into a block-local one; the element offsets differ per operand, the logical row does not. Its tests pin both properties the design rests on, for the measured case's own axis record: consecutive rows are one source run (8 f64 = one 64-byte line, so the read stops touching one element per line), and every one of those rows carries all 48 `c` values whose destination offsets are consecutive (the run the emitter writes). Those are exactly the two runs the earlier drafts could not provide. Nothing calls the module yet, which is deliberate: it is the behaviour-preserving first step, and the enumerator that consumes it comes with the next. Gate green: `cargo fmt --all -- --check`, `cargo clippy -p tprims-contract --all-targets -- -D warnings` with and without `--features phase-timing`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo doc --workspace --no-deps` with `-D warnings`, `cargo test --workspace` (80 test binaries, all ok). Local branch only, not pushed: this is scaffolding for an architectural change, not a deliverable on its own.
…hat removes the hard half Second step of `docs/design/blocked-outer-traversal.md`, still unused by the driver and so behaviour-preserving: - `Role` and `role()` map the plan's oriented row role to the two strides the blocked path needs (operand and output), so the axis-order question is answered once, in the orientation the kernel will actually use. - `blocked_eligibility` is the conservative predicate the reviews asked for: the role's fastest axes must disagree **and** the contraction must fit one K slab and one NC panel. The second half is new and it is what makes the change small - with one slab and one NC panel a block's accumulator is complete inside the single `pc` iteration the driver already performs, so there is no cross-slab persistence, no ownership or barrier restatement, and no B-publication lifetime to specify. - `output_order` and `reorder_rows` are the other half of the mechanism: visiting a block's rows in output-offset order makes the stores consecutive (measured: runs of 48, one per `a`, and the ties keep the block's own order, which the raw write-back contract's repeated-address behaviour depends on). Tests pin both properties the design rests on, for the measured case's own axis record: the block holds a one-line source run of 8 `f64` **and** all 48 `c` values whose output offsets are consecutive; eligibility is refused for two K slabs, two NC panels, a swapped role, a single live axis, and a role whose fastest axis agrees. Gate green: `cargo fmt --all -- --check`; `cargo clippy -p tprims-contract --all-targets -- -D warnings` with and without `--features phase-timing`; `cargo test --workspace` (80 test binaries, all ok). Local branch only, not pushed.
…ng meet
Third step of `docs/design/blocked-outer-traversal.md`, still behaviour-preserving: the
eligibility is now computed in execution initialization, where the plan's oriented role
axes and the resolved `mc`/`kc`/`nc` are both in scope, and reported under the phase
instrument (`TPRIMS_PHASE=1`) until the enumerator consumes it. No path changes: the
driver still walks today's consecutive slices.
Checked on real plans, which is the point of landing it here rather than in a test:
| case | eligibility |
| --- | --- |
| `abjc-cbka-kj` f64, the case that loses 1.5x | `Some(BlockShape { span: 0, line: 2, line_len: 8 })` - the output's fastest axis is `c`, the operand's is `a`, and a line is 8 `f64` |
| `abcijk-eiac-jkeb` f64, where this library wins 2.5x | `None` |
| `ijkl-imjn-lnkm` f64 | `None` |
So the predicate selects the measured class and nothing else among the cases checked,
and it agrees with the unit test's synthetic record (the two differ only in how the axes
are listed: the plan lists them output-fastest first, `(c, b, a)`, the test `(a, b, c)`).
Gate green: `cargo fmt --all -- --check`; `cargo clippy -p tprims-contract --all-targets
-- -D warnings` with and without `--features phase-timing`; `cargo test --workspace` (80
test binaries, all ok); `cargo doc --workspace --no-deps` with `-D warnings`.
Local branch only, not pushed.
The data side of the blocked outer traversal. The blocked path computes a block's micro-tiles in the packed operand's row order (the order the packer can read without touching one element per cache line) while the emitter writes `D` fastest along the output's axis, so the block's tiles have to be reordered once, as a block, before emission. Reordering a single 24-row tile is a no-op on the shapes this exists for, which is what the earlier designs got wrong. `pack::permute`: - `slots` gives the real positions one logical element occupies in a tile of each `TileFormat`, mirroring `tile_value`'s addressing **without recombining planes**: `Real` one, `Planar`/`Interleaved`/`OneM` two, `FourM` four, `ThreeM` three (the doc's own counts; there is no `OneE` tile format). - `permute_grid_rows` moves whole rows between grids of `mr x nr` tiles, taking each destination row from any source tile. Because it copies *slots*, every plane layout survives bit for bit and conjugation stays where it already is - at the emitter, which applies it while writing `D`. Tests: the identity map is bit-identical for all six formats; a row pulled from another tile lands in the destination row and no destination slot is left unwritten; and `slots` is shown to cover every position of a tile exactly once per format, which is what makes the copy bit-exact rather than merely approximate. Gate green: fmt, clippy (workspace, with and without `--features phase-timing`), `cargo test --workspace` (80 test binaries, all ok), doc with `-D warnings`. Local branch only, not pushed.
…rsal Steps 1-4 (row views, collect instead of emit, the blocked branch, the tile-grid workspace), each with the signatures it changes and the gate it lands on, so the remaining work is mechanical rather than a matter of taste. Step 1 is the identity view the reviews approved: static_grid.rs slices the global vectors as today, so it changes no behaviour and partition_bitwise pins it.
…ur-preserving) First of the four wiring steps in `docs/design/blocked-outer-traversal.md`, and the identity view the reviews approved: `pack_a_rows` and `compute_block` now take the row scatters they use as slices with a live count instead of `ic`/`ic_len` into the epoch's global vectors, and index them from zero. Both schedulers pass exactly today's interval - the epoch's own slice and its block-scatter twin - so no behaviour changes; the blocked path will pass a gathered block-local list through the same parameters. Two things this surfaced, both worth keeping: - `C`'s block scatter is **deliberately empty when `beta == 0`** (`driver/mod.rs` makes the range `(len, len)`), and the write-back reads it with `.get()`, so empty is a signal and not a range to cut: the caller passes the empty slice through unchanged. - The C ABI tests load a *prebuilt* `libtprims.so`, so `cargo build -p tprims-capi` has to run again after a library change before `cargo test --workspace`; a stale library is what the first failures were, not the refactor. Verified behaviour-preserving, by tests and by measurement: - `cargo test --workspace` passes: 80 test binaries, all ok (after rebuilding `tprims-capi`), including `partition_bitwise`, which pins serial/static/dynamic/width equality. - The measured case's phase profile is unchanged: `total 43.13 ms, pack_a 27.60 (64.0%), kernel 14.29, writeback 1.79`, against the pre-refactor `43.10 / 27.6 / 14.3 / 1.8`. - fmt, clippy (workspace, with and without `--features phase-timing`) and doc with `-D warnings` are green. Local branch only, not pushed.
`block::rows` takes the block's per-axis origin instead of two scalars, and `block::block_origins` enumerates every origin a role needs: the span axis once (a block takes it whole), the line axis in steps of a line, every other axis in steps of one. The test that matters is the one the fourth review asked for: for the measured case's own axis record the block rows, concatenated over all 240 origins (40 `b` x 48/8 `a`), are a permutation of `0..92160` - no gaps and no repeats. Coverage is what the driver's gates will check on real calls, and it is now checked on the enumeration itself, before any wiring. Gate green: 24 `tprims-contract` lib tests, `cargo test --workspace` (80 binaries, all ok), fmt, clippy (workspace, with and without `--features phase-timing`). Local branch only, not pushed.
The campaign's tcbench cell carries TBLIS per case, so the reference's own applicability range can be measured rather than assumed. Over 196 rows it wins 16, and 12 of those 24 rows are the ones with a thin output and a shallow K (`n <= 64 and k <= 48`, median ratio 0.99); outside that class it wins 4 of 168. The reason matches this design's: a long output-fastest axis against a short operand-fastest axis is exactly where the pack read touches one element per line. The same measurement says the current implementation is not ready to be a default: with the switch on it helps 2 of 98 one-thread rows and hurts 94, median 25% slower, the two wins being the two thinnest cases. The mechanism is measured (pack_a 27.6 to 5.5 ms); the per-block allocation churn (~9 ms of a 23 ms call) and the block-level transpose are what eat it elsewhere. Promotion therefore needs both the thin/shallow rule and the allocation fix, and the switch stays off until then.
`run_block` built about ten `Vec`s per block, which cost more than the blocked path saved. They are now stack arrays bounded by `BLOCK_ROWS_MAX` (= `BLOCK_MC_BUDGET`), with the boundary asserts that make the bound true; the workspace request keeps the two grids and the no-workspace path now sizes its panel for them as well. Two correctness bugs fell out of running the whole corpus with the switch on: the row map must be sorted over `live` rows and padded afterwards (a `live % mr != 0` shape indexed past the end of `dm_rows`), and the grid slot must be the global column tile or column-group workers overwrite each other. Both are fixed; the workspace suite is 80/80 again. With that, the blocked path is cost-neutral over the corpus (8 helped, 15 hurt, 75 neutral, median x1.00, from 2/94/2 and x0.80), and the measured case goes 43.4 to 31.3 ms, 11.2 GF/s against 8.2. Promotion still waits on a profitability test, and the geometry does not supply one: `adbjc-cbdka-kj` (x1.22) and `abjcd-dkbac-jk` (x0.74) agree in `span`, `line`, `line_len` and `grid_elems`. The next measurement is the operand's scatter regularity inside a block, not another guess. The switch stays off and this stays out of a PR until it improves a published number.
Per case, the eligible role's strides differ even when `BlockShape` does not, but the phase instrument locates the loss away from the mechanism: `pack_a` falls exactly as designed (35.3 -> 11.2 ms, 26.1 -> 15.8 ms) while the unmeasured remainder - the block's gathers, scatters and the grid -> grid2 permutation, which is a second pass over the block's output - is 9 ms on the two shapes that win and 20 ms on the four-axis shapes that lose. Profitability is therefore about work per block (block rows times `n` against a fixed per-block cost), not about line geometry, and the next descriptor to record is the block count and the block's output size. The diagnostic also now prints the role axes' strides, which is what showed the two indistinguishable shapes apart; it stays behind `phase-timing` + `TPRIMS_PHASE`.
…hole answer Three candidate explanations for the blocked path's remaining cost are now ruled out by measurement: it is not the instrument's accounting (15 ms at `--prime-ms 0` and at `--reps 20`), it is not the grid -> grid2 permutation (removing that copy entirely - each panel tile now emits with its destination tile's own rows and scatters, and the permutation only runs when a panel tile's rows are not a whole destination tile - changed `setup` from 14.7 to 15.0 ms), and it is not per-block metadata (the eligibility divided the block budget by the other axes as well as the span axis, so blocks were ten times too small and `line_len` was 1 instead of the design's 8; fixing it made the blocks bigger and `setup` bigger with them). What remains is the work the instrument does not attribute, and the one candidate that scales with it is the write-back: with the block's rows gathered in the operand's order the loads become contiguous (`pack_a` 35.3 -> 5.3 ms) and the stores become the strided side. The path is a trade of strided loads against strided stores, so it pays only when `m*k > m*n`. Fitted to the corpus the best rule (`n <= 64` and `k > n`) selects 6 rows, helps 3, hurts 1, and a median 2.4%. That is not a promotion, so the switch stays off and the objective is not met; the design document records why.
…names The blocked path's emit loop was in no phase (phase 3 sits inside the unblocked write-back), so its whole cost landed in the derived `setup` remainder, and per-block `Instant::now()` calls - the obvious way to split it - are expensive enough on this host to swamp what they measure (removing the sort entirely left the "metadata" number unchanged at 21.7 ms). Two measured phases instead, taken once per call: `block_meta` for the per-block gathers, scatters and row order, `permute` for the grid permutation and the output-ordered rows and scatters. The parts now add up to the call (38.6 against 38.1 ms): pack_a 6.3 against 28.8, kernel-own 0.8 against 11.7, stores 11.0 against 10.6, plus 15.0 of block metadata and 5.5 of permutation - a net 14 ms win on the measured case, with the remaining cost named and bounded in the design document.
The blocked outer traversal is now selected by profitability rather than by geometry
alone. It buys a contiguous pack and pays for that with a per-block grid permutation
and per-block metadata; over the 98 tcbench rows at 1T, measured with the path on and
off in the same session, the class where that trade comes out ahead and never behind
is a thin output against a deeper contraction:
n <= 64 and k > n: 6 rows, 4 wins, 0 losses, 2 neutral, median x1.143
(abjc-cbka-kj f64 42.0 -> 25.6 ms, x1.64;
adbjc-cbdka-kj f64 46.2 -> 32.6 ms, x1.42)
everything else: today's traversal, unchanged - `grid_elems`, `block_rows` and
`mc` all take their old values when the eligibility says no.
That class is also where the reference's own advantage lives (TBLIS wins 12 of its 24
`n <= 64 and k <= 48` rows and 4 of 168 elsewhere), so this closes the gap exactly
where this library was furthest behind.
Two geometry bugs came out of the same measurements and are fixed here: the block's
row count multiplied the *other* axes in as well, so `MC`, the packed panel and the
tile grid were sized for a block many times larger than the one `block::rows` builds
(and the eligibility divided the block budget the same way, which is why blocks were
ten times too small and `line_len` was 1 instead of the design's 8); and the row map
must be sorted over `live` rows and padded afterwards, or a `live % mr != 0` shape
indexes past the end of the block's rows.
Gates: workspace tests 80/80, fmt, clippy with and without `phase-timing`, rustdoc,
and the harness oracle (`verify`) at 16 MiB 1T and 4T and at 1 MiB, all within
tolerance.
… no allocations An independent review of the finished diff (gpt-6.1-sol) found two memory-safety blockets and they were right. * A Direct family wrote `D` instead of its tile: `direct_tile` never tested whether the call was collecting, so an eligible call could compute into `D`, leave its grid slot untouched, and then emit that slot. The guard now tests `collect` - the invariant itself - and the eligibility refuses Direct families outright, as the design always claimed. * The permutation read grid slots no worker had written: a column-group worker fills only its own column tiles while `permute_grid_rows` walks every column. The eligibility now requires `pm == 1 && pn == 1`, so the blocked path is serial, which is where it was measured; partitioning the block itself stays a separate change. * The row budget was not enforced: `(mc_budget / span).max(1)` admits a span larger than the budget, and the panel, the grid and the row map are sized from the block rounded up to whole `MR` tiles. Both are conditions in the eligibility now, with tests, instead of panics in `run_block`. * The traversal allocated: `role`, `block_origins` (a `Vec<Vec<usize>>`, one allocation per block) and `rows` allocated per execute, against the workspace's no-allocation contract. They are fixed-size scratch now (`role_into`, `Origins`, `rows_into`), and the incremental origins are checked against the materialized list by a test. * The two extra phases are gone again: `block_meta` contained `pack_a` and `kernel`, per block scopes cost more than they measured, so the instrument is back to four phases plus the `setup` remainder, and the design document says what that remainder can and cannot support. The direct-emit variant that never triggered on any measured case is deleted, `permute::slots` is private, and `permute_grid_rows` no longer takes the plane count it can derive. Measured again after the fixes, on the same 98 tcbench rows at 1T: `abjc-cbka-kj` f64 42.0 -> 24.7 ms (x1.70), `adbjc-cbdka-kj` f64 46.2 -> 28.4 ms (x1.63), the c64 rows 1.21x and 1.15x, and no eligible row slower. The rows that read 3-7% worse are ineligible, take an identical code path in both versions, and are drift. Gates: fmt, clippy (with and without `phase-timing`), rustdoc, 80/80 test binaries, and the oracle at 16 MiB 1T and 4T and at 1 MiB, all within tolerance. Not verified: `tests/packed_workspace_alloc` exercises an ordinary matrix multiplication, whose one folded row axis can never select this path, so the zero-allocation claim rests on reading the code rather than on that test. Recorded in the design document.
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.
What
A blocked outer traversal for the packed driver: a block spans the output's fastest
axis whole times a line's worth (8 f64 elements) along the operand's fastest axis, its
tiles are permuted into the output's order, and it is emitted once. Where the packed
operand and the output disagree about their fastest axis, today's slices let no
reordering produce a contiguous destination run and the pack reads one element per cache
line; a block that spans whole runs of both axes fixes that.
The path is taken only where it is measured to pay:
n <= 64 && k > non top ofk <= KC,n <= NC, one K slab, one NC panel,pm == 1 && pn == 1and a non-Directfamily. Everything else keeps today's traversal, with
grid_elems,block_rowsandmctaking their previous values.
Evidence (98 tcbench rows, 16 MiB, 1T, path off and on in one session)
abjc-cbka-kjadbjc-cbdka-kjadbjc-cbdka-kjabjc-cbka-kjNo eligible row is slower; the rows that read 3-7% worse are ineligible, take an
identical code path in both versions, and are drift. The class is also the one where the
reference wins: over the campaign's tcbench cell TBLIS wins 12 of its 24
n <= 64 && k <= 48rows and 4 of 168 elsewhere.Mechanism, from the
phase-timinginstrument:pack_a28.8 -> 6.4 ms, against about8.6 ms of added permutation and per-block work.
Review
The finished diff was reviewed independently (
gpt-6.1-sol, read-only, full sources).It found two memory-safety blockers, both real and both fixed here:
direct_tiledid not testcollect, so a Direct family could compute intoD, leaveits grid slot unwritten, and have the emitter read it. The guard now tests
collect,and the eligibility refuses Direct families.
only its own: the eligibility now requires
pm == 1 && pn == 1, so the path is serial.It also found the missing row-budget bound (now a condition, with tests), per-execute
allocations (
role,block_origins,rows- now fixed-size scratch) and anon-disjoint phase account (the extra phases are removed and the design document says
what
setupcan and cannot support). The never-triggering direct-emit variant isdeleted,
permute::slotsis private, andpermute_grid_rowsno longer takes a planecount it can derive.
Not verified
tests/packed_workspace_allocexercises an ordinary matrix multiplication, whose onefolded row axis can never select this path, so the zero-allocation claim rests on reading
the code. The path is serial-only, so 4T benefits are not claimed. Full record:
docs/design/blocked-outer-traversal.mdsections 9-14.Gates
cargo fmt --all -- --check,cargo clippy --workspace --all-targetswith and withoutphase-timing(-D warnings),RUSTDOCFLAGS="-D warnings" cargo doc --workspace --no-deps,cargo test --workspace(80/80 binaries), and the harness oracle(
tcbench verify, f64 and c64) at 16 MiB 1T, 16 MiB 4T and 1 MiB - all "all comparisonswithin tolerance".