Add Matrix::transpose - #175
Conversation
|
Warning Review limit reached
Next review available in: 56 minutes Limit details: You’ve used all 1 included review currently available under your plan. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdds a reusable 64×64 lane-wise bit transpose. ChangesBinary Matrix Transpose
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to This PR adds matrix transposition with a localized implementation and documented coverage across supported primes and boundary shapes; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant Matrix
participant ScratchTiles
participant transpose_lanes
Matrix->>ScratchTiles: Gather padded binary matrix tiles
ScratchTiles->>transpose_lanes: Transpose each 64×64 tile
transpose_lanes-->>ScratchTiles: Return transposed tile
ScratchTiles-->>Matrix: Write valid destination rows
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Entry (i, j) of a p = 2 matrix is bit j % 64 of limb j / 64 of row i, so a 64-square block of entries is one limb from each of 64 consecutive rows. `blas::block::transpose_lanes` moves such a block with six masked shift-and-XOR passes (the recursive delta swap of Hacker's Delight 7-3) instead of 4096 single-bit extractions. Two things beyond the swap matter about as much again. Taking 8 blocks at a time gives each destination row 8 consecutive limbs — a whole cache line, where a block at a time deposited 8 useful bytes per line touched. And holding one block per lane means every lane runs the same swap on the same row indices, so nothing moves between lanes and the inner loop is elementwise; that is adapted from mkarppa/matmul (MIT; Karppa & Kaski, arXiv:1909.01554), which transposes over uint4 rather than a word at a time. `Matrix::transpose` works an 8x8 tile of blocks through a 32 KB scratch, so both trips to main memory are contiguous and the shuffling stays in L1. Odd primes use the entry-at-a-time path. Against that path: 156x/125x/170x/181x/180x at 512/1024/2048/4096/8192 square, effective bandwidth flat at 7.4-9.4 GB/s. Measured on a dev box, not the machine in the benchmark suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EFfXYt9zvAvHWhNHnULe6r
Matrixhad no transpose. This adds one, blocked atp = 2.Entry
(i, j)of ap = 2matrix is bitj % 64of limbj / 64of rowi, so a 64-square blockof entries is exactly one limb from each of 64 consecutive rows.
blas::block::transpose_lanesmoves such a block with six masked shift-and-XOR passes — the recursive delta swap of Hacker's
Delight 7-3 — rather than the 4096 single-bit extractions an entry-at-a-time transpose performs.
Two things beyond the swap itself matter about as much again:
a block at a time touched 64 cache lines to deposit 8 useful bytes each. Taking 8 blocks at a time
makes those contributions 8 consecutive limbs of each destination row — one whole line.
moves between lanes and the inner loop is a straight elementwise operation. This is adapted from
mkarppa/matmul (MIT; Karppa & Kaski,
arXiv:1909.01554), which transposes bit blocks over
uint4rather than one word at a time; one block per lane is the CPU form of that width.
Matrix::transposeworks an 8×8 tile of blocks through a 32 KB scratch, so both trips to mainmemory are contiguous and the shuffling stays in L1. Odd primes keep the entry-at-a-time path.
Against that path, on square matrices:
Effective bandwidth is flat at 7.4–9.4 GB/s across that range. Measured on one dev box; it is a
cache-sensitive routine, so expect the numbers to move with the machine — beyond ~8192² the working
set leaves L3 and throughput falls off.
Tests
transpose_lanesagainst an entry-at-a-time oracle: random blocks, involution, and everysingle-bit block.
Matrix::transposeunder proptest at all primes (via the existingArbitrary for Matrix), plus64 hand-picked shapes straddling block boundaries in each dimension independently — 1, 63, 64, 65,
127, 128, 129, 200 in each axis.
History
This branch previously held a naive entry-at-a-time transpose. It has been rewritten rather than
built on: the old implementation predates the
Matrixrewrite and assumedvectors: Vec<FpVector>,where
Matrixnow storesdata: AVec<Limb>with a stride, which is what makes the blocked formpossible at all.
Split out of the work in #285; the GPU half is a separate PR and does not depend on
this one.
Summary by CodeRabbit
New Features
Tests