ROCm: map the executing lane frontend-to-hsaco, and price W1.1's adoption cost - #522
Merged
Merged
Conversation
…tion cost
W1.1 has been sized as "compiler-contract work on the Tile IR" without anyone
stating which executing lane traverses Tile IR. Measured it. The answer changes
the sequencing, so it gets its own reference doc rather than a paragraph.
Two GEMM lanes exist and only one runs:
* Lane A, the DIRECTIVE lane, is production (11 callers). Python composes a
one-op MLIR string that is ALREADY at Target IR —
`"tessera_rocm.wmma_gemm"() {...}` — and the pipeline is
generate-wmma-gemm-kernel → lower-tile-to-rocm → lower-tessera-target-to-rocdl
→ gpu-to-rocdl → hsaco. No Graph IR, no Schedule IR, no Tile IR.
* Lane B, the CANONICAL lane, does start from `tessera.matmul` Graph IR and runs
tessera-tiling + tessera-tile-ir-lowering. It has ONE caller in the whole
tree, and it is `benchmarks/rocm/benchmark_rocm_canonical_gemm_kloop.py`.
So the Graph-IR lane has zero production callers, and Schedule IR is absent from
every pipeline (0 references; TesseraPM is not linked into tessera-opt), which
matches the plan's own note that those passes are annotation-only skeletons. On
the lane that runs, the "four-layer stack" is one layer: Target IR.
The expander population is the part that prices the remaining work:
Generate*.cpp in the ROCm backend 71
distinct generate-* passes the runtime drives 58
expanders consuming tile.view / tile.fragment_pack 0
runtime pipelines including lower-tile-to-rocm 4 of 9
Consequences, stated plainly because the plan currently implies otherwise:
* W1.1 affects NO executing ROCm kernel today. `lower-tile-to-rocm` is in the
production pipeline and is a verified no-op there — runtime.py's own comment
records byte-identical hsaco with and without it. That is a capability with no
producer, the shape Decision #29 exists to flag.
* Step 3 migrates 1 of 58 expanders. Step 5 (delete the permissive branch) sits
behind the other 57, not behind two more numbered steps.
* The W1.1 row's 5w covers BUILDING the contract, not ADOPTING it, and adoption
scales with the expander population.
This does not make W1.1 wrong: a composable typed lowering is a precondition for
any adoption, and steps 0/3a removed two real blockers. It does mean the
adoption policy is an unpriced project-endpoint decision, so the doc names three
options (migrate all 58 / migrate the performance-critical family only / treat
Tile IR as an optional lane) rather than assuming the first.
Linked from the ROCm queue and from the plan's open-work list, so it is reachable
from the surfaces Decision #26 says to start at.
Docs only. 14448 unit passed, ruff clean, 24 generated docs in sync.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95860c9d69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both P2s were right, and the second one mattered more than the first.
1. The pipeline coverage numbers were substring counts, not pipeline entries.
I published "lower-tile-to-rocm 4 of 9" and "lower-tessera-target-to-rocdl
8 of 9" from `grep -c` over runtime.py. That also matches the long
explanatory COMMENTS next to the two GEMM builders. Counting actual pipeline
entries — a quoted, comma-terminated pass name inside a pipeline string —
gives 2 and 5. The denominator (9 pipelines terminating in
gpu-module-to-binary) is unchanged.
The correction strengthens the finding rather than weakening it: only TWO of
nine runtime pipelines reach lower-tile-to-rocm at all, and both are the GEMM
lanes. Re-run commands are in the doc so the next reader does not repeat the
substring mistake.
2. Step 5 is not gated by the expander population — and I contradicted a
scoping the plan already had right.
I wrote that step 5 "sits behind the other 57" expanders. That conflated two
independent costs. `MMAOp::verify()`'s permissive branch governs only
producers that emit `tile.mma` with bare fragments, and those are enumerable:
5 creation sites in 4 files —
GenerateWMMAGemmKernel.cpp:465 (only when via-tile=true)
GenerateWMMALinearAttnKernel.cpp:143 (only when via-tile=true)
GenerateWMMAFlashAttnKernel.cpp:230 (only when via-tile=true)
TileIRLoweringPass.cpp:924, :997 (the Graph-IR / Lane B path)
plus the Python emitters. That is exactly the "five construction sites plus
Python emitters" W1.1 already scoped. An expander that never emits tile.mma
cannot block deleting a tile.mma verifier branch.
The doc now keeps the two costs apart explicitly:
close the Tile fragment contract (steps 3-5) | 5 C++ sites + Python emitters
make the ROCm backend go through Tile IR | 58 expanders, unpriced
Only the second scales with the expander population. My version would have
deferred a closeable contract cleanup behind unrelated codegen — the opposite
of what the measurement supports.
Both corrections propagated to the two summaries that quote these numbers
(rocm/todo.md and the plan's open-work list), so the wrong framing does not
survive in the surfaces Decision #26 says to start at.
Docs only. 14448 unit passed, ruff clean, 24 generated docs in sync.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Docs only. W1.1 has been sized as "compiler-contract work on the Tile IR" without anyone stating which executing lane traverses Tile IR. I measured it. The answer changes the sequencing, so it gets its own reference doc.
Two GEMM lanes exist; only one runs
Lane A — the directive lane. This is production (11 callers). Python composes a one-op MLIR string that is already at Target IR:
No Graph IR. No Schedule IR. No Tile IR.
Lane B — the canonical lane. Starts from real
tessera.matmulGraph IR, runstessera-tiling+tessera-tile-ir-lowering. It has one caller in the whole tree, and it isbenchmarks/rocm/benchmark_rocm_canonical_gemm_kloop.py.So the Graph-IR lane has zero production callers, and Schedule IR is absent from every pipeline (0 references;
TesseraPMisn't linked intotessera-opt) — consistent with the plan's own note that those passes are annotation-only skeletons. On the lane that runs, the "four-layer stack" is one layer: Target IR.The expander population prices the rest
Generate*.cppin the ROCm backendgenerate-*passes the runtime drivestile.view/tile.fragment_packlower-tile-to-rocmWhat follows — stated plainly, because the plan implies otherwise
lower-tile-to-rocmis in the production pipeline and is a verified no-op there —runtime.py's own comment records byte-identical hsaco with and without it. A capability with no producer: the shape Decision Apple GPU MLA: native f16/bf16 for the absorbed decode kernel #29 exists to flag.This doesn't make W1.1 wrong — a composable typed lowering is a precondition for any adoption, and steps 0/3a removed two real blockers. It means the adoption policy is an unpriced project-endpoint decision, so the doc names three options rather than assuming the first:
(a) migrate all 58 — the only option that makes the layered-stack claim true for ROCm; (b) migrate the performance-critical family only (GEMM, flash-attn, linear-attn), documenting the long tail as direct Target-IR expanders; (c) treat Tile IR as an optional lane for ops that benefit from shared tiling/pipelining.
Decision #28's three-tier model is compatible with (b) and (c). The current plan text implies (a) without pricing it. That's your call, not mine — the doc lays out the costs and stops.
Also flagged: Lane B is compiled, exercised only by a benchmark, and needs its own decision — canonical front door, declared oracle with a differential test (Decision #31), or not carried.
Linked from the ROCm queue and the plan's open-work list so it's reachable from the Decision #26 entry points. Every number has its re-run command in §6.
Gates
14448 unit passed · ruff clean · 24 generated docs in sync
🤖 Generated with Claude Code