TileOP: enforce the ISA capacity-derived TCVT geometry contract (#42) - #97
Merged
Conversation
The ordinary (non-CUBE) TCVT path required bitwise-identical C++ Rows/Cols between source and destination. That is stricter than the ISA: PTO-TILE-TCVT legality requires equal Row, Col, ValidRow, ValidCol where Row/Col are the capacity-derived physical dimensions (DerivedTileRows = capacity*8 / (Col * elementBits), asl/tile/model/shape/rows-columns.asl), and the capacities independently match each side's own DataType. For a narrowing conversion (bf16/fp32 -> e8m0 [Rows,1]) the 128B minimum TSize doubles the destination's derived rows (e.g. 64 -> 128), so the [Rows,1]/[Rows,1] encoding is illegal under the ISA and the emulator rightly rejects it (issue #42). The ISA-legal encoding declares a wider physical column on BOTH sides (e.g. [64,2] with ValidCol=1: bf16 256B -> 64 derived rows, e8m0 128B -> 64 derived rows). This replaces the bitwise Rows/Cols assertion with the exact ISA condition (equal capacity-derived rows), keeping ValidRow/ValidCol equality, and documents the narrowing pattern in the TCVT usage page. Verified: - issue #42 chain (half->fp32->bf16->e8m0, all [64,2] VC=1) compiles; the final bundle emits lb2=2 with the e8m0 destination at 128B (DerivedTileRows 64 == source 64) - the old [64,1]->[64,1] form is now rejected with a static_assert message that names the ISA contract and the [Rows,2] fix - full gate 64/74 (11 failures identical on baseline); unittest 40/40
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.
Fixes the TileOP side of #42 per the ISA verdict in the earlier comment.
What changed
The ordinary (non-CUBE) TCVT path required bitwise-identical C++
Rows/Colsbetween source and destination. That is stricter than the ISA:where Row/Col are the capacity-derived physical dimensions (
DerivedTileRows = capacity*8 / (Col * elementBits),asl/tile/model/shape/rows-columns.asl) — not the C++ logical Rows/Cols — and each side's capacity independently matches its own DataType.Why the old check was wrong for narrowing dtypes
bf16 → e8m0
[64,1]: the e8m0 destination's 64B payload is padded to the 128B minimum TSize, so its derived rows double (128) while the bf16 source stays at 64 — the emulator correctly rejects this per the ISA (that is the crash in #42). The ISA-legal encoding declares a wider physical column on both sides:0 ""
0 ""
0 ""
1 "/usr/include/stdc-predef.h" 1 3 4
0 "" 2
1 ""
The old bitwise assertion made this legal encoding un-expressible (compounded by the reduce-chain pinning Col=1).
The fix
Rows==Rows && Cols==Colswith the exact ISA condition: equal capacity-derived rows (computed from each side'sStorageBytes— the B.IOT-encoded capacity — physical Col, and element bits), keeping ValidRow/ValidCol equality.[64,1]→[64,1]e8m0) now fails with a static_assert that names the ISA contract and points to the[Rows,2]pattern.Verification
[64,2] VC=1) compiles; final bundle:lb2=2, e8m0 destination at 128B, derived rows 64 == source 64Note: the kernel-side fix (dynamic_mx_quant) needs to declare the scale-chain tiles as
[TileM, 2]with ValidCol=1 per the documented pattern; the model needs no change (its assertion is the correct ISA behavior).