record(GEMMA4-ROCM-KEEP): measured SharedK-WMMA plateau on 2x R9700 - #676

Thanks for this — and welcome. Reviewed as part of a sweep over the open external PRs.

Before the findings, the part that matters: all eight checkers you list as passing locally do pass — I ran every one at your head SHA and confirmed it. Everything below is something no checker in this repo looks for, so none of it is a diligence failure on your part. Several of your calls are ones this project learned the hard way: disclosing the dropped 1170 first-rep with its cause rather than quietly taking the median, per-depth rep counts, explicitly writing "does not claim that bar", refusing to name an LLVM defect from a spill count, and not touching STATUS.md when no lifecycle moved. The RADV/ACO matched-instrument comparison in your second commit is the most reusable thing in the PR.

The blocker is that the recipe cannot be reproduced from main.

Four of the decode knobs are read by no production code in this tree:

git log -S'VT_ATTN_DECODE_KV_SPLITS' and git log -S'SLIDE_WARPS' both put the production consumers on dd1288095, whose subject line is "wip(lab): decode/BPE/structural-tags stack (not for mudler merge)".

Meanwhile every prefill item — SharedK-WMMA on, FLASH off, GEMM_M=2048, PEER_ACT=1, batch MoE T>=64 — is already the shipped default on main. So read against this tree the recipe is the default configuration plus four inert strings, and docs/USAGE.md:2125 now tells readers of a public page to set two variables that do nothing.

I don't think the numbers are wrong. The likely story is honest measurements of your working branch written up against the wrong baseline. But a record whose whole value is reproducibility has to name the tree it describes, and the spec header still says PR tip feat/gemma4-rocm-fp8-split with no SHA. Either state the exact revision and mark the decode knobs lab-branch-only, or drop them and re-state the recipe as "the defaults" — which on main it is.

Why this was easy to miss, and not your fault: test_gemma4_rocm_fp8_seams.cpp:69 asserts EnvInt("VT_ATTN_DECODE_KV_SPLITS", 16) == 16 with the variable unset. That is default == default — it tests the helper's fallback, not the product, and would pass identically if the env var had never existed. Its own comment half-concedes it. That is a pre-existing defect of ours and I'm filing it separately; it is why three non-existent knobs look real from inside the repo.

Second blocker: no issue. AGENTS.md wants one linked in three places that agree — the roadmap issue table, the row's spec, and the PR body. The spec header cites (#317), which is a merged PR rather than an issue; the roadmap has no entry; the body has none. Also grep -rn "GEMMA4-ROCM-KEEP" matches nothing in the tree, so the row ID your branch name asserts doesn't exist yet.

On the denominator. The comparison arm is "Vulkan Q8" with the stack unnamed. The prior entry for this same box labelled it llama.cpp Vulkan Q8 … separate stack (.agents/benchmark-record.md:16185), and docs/ROCM.md:300-303 says of this exact milestone: "The bar is vLLM, not llama.cpp." It's also not quant-matched — ours FP8, the bar Q8_0 — while derived ratios (~1.74x / 2.47x) are published across that boundary.

Your "does not claim that bar" framing is the right instinct and it is one sentence from being fully admissible. There's a template already in the tree at docs/BENCHMARKS.md:37: "no vLLM denominator (pin lacks muse_glimmer); SECONDARY llama.cpp, same GGUF, idle GB10 | vLLM axis is an OPEN GAP." Name the stack, label it secondary, declare the vLLM axis open. Note docs/ROCM.md:216-231 documents two working vLLM-ROCm oracle recipes on RDNA4, so if it turns out vLLM genuinely can't serve this checkpoint on 2x R9700, that's worth saying explicitly rather than leaving unstated.

Placement. The measurement landed on USAGE.md, ROCM.md, FEATURES.md and the spec, but not on either surface this project keeps measurements on. .agents/benchmark-record.md is append-only — no merge-lock excuse — and its header says "read it before re-running a lever." Your five rejected levers are exactly what it exists for, and they aren't in it. docs/BENCHMARKS.md:452 still reads PENDING: no binding throughput number with no reconciliation.

docs/FEATURES.md:335 is the sharpest one: the edit removes Env VT_GEMMA4_*/VT_ATTN_*, seam test_gemma4_rocm_fp8_seams — the two facts that belong on a feature surface — to make room for a bare absolute with no denominator and none of your own caveat. The only other throughput in that table carries its vLLM ratio.

Two smaller things worth having:

• docs/ENVIRONMENT.md:198 documents GEMM_M default 256, range 16..2048. The code says default 2048, range 16..8192 (gemma4_moe.cpp:1016-1021). Your new sentence reads as a lab override of a documented 256 when 2048 is what the binary already does. Worth correcting in the same edit — check-env-doc.py verifies documentation exists, not that it's accurate.
• The ~18k and ~42k rows don't state VT_GEMMA4_GPU0_HEADROOM_GB, and docs/ENVIRONMENT.md:194 records for this exact board that 8 survives 16k+ prefill while 6 OOMs at ~11k. Either the default was used, or the recipe is incomplete for its two deepest rows.

Please keep the residual section as-is. The HIP cm1 339-spill vs ACO 0-spill contrast on the same silicon, with the explicit refusal to call it a named defect, is disciplined work and it should not be lost when the spec is compacted — I'd suggest an issue so it survives independently.

Decode regime would help too: 55.5 / 49.1 t/s with no concurrency, depth or batch size can't be compared to anything, including a future vLLM leg. Your prefill depth curve is exactly right; decode just needs the same treatment.

Nothing here needs new measurements. Happy to look again once the recipe names its tree.

Following up on the review above — there is one question only you can answer, and it blocks the rest.

The question: which tree produced these numbers?

The KEEP recipe lists four decode knobs, and none of them is read by production code on main:

git log -S puts their production consumers on dd1288095, whose own subject line is "wip(lab): decode/BPE/structural-tags stack (not for mudler merge)".

Meanwhile every prefill item — SharedK-WMMA on, FLASH off, GEMM_M=2048, PEER_ACT=1, batch MoE T>=64 — is already the shipped default on main.

I am not suggesting the measurements are wrong. The most likely explanation is that they are honest numbers from your working branch, written up against main as the baseline. But a record whose whole value is reproducibility has to name the tree it describes, and the spec header still says PR tip feat/gemma4-rocm-fp8-split with no SHA.

So: which revision were the 2112 / 2014 / 1705 / 1099 prefill figures and the 55.5 / 49.1 decode figures taken on? Once that is stated, there are two clean ways forward and either is fine:

• State the SHA and mark the decode knobs lab-branch-only, so a reader knows the recipe is not reproducible from main yet; or
• Drop the four decode knobs and re-state the recipe as "the shipped defaults", which on main is what it is.

The practical urgency is that docs/USAGE.md:2125 currently tells readers of a public page to set two variables that do nothing on main.

Not your fault, and worth saying

tests/vt/test_gemma4_rocm_fp8_seams.cpp:69 asserts EnvInt("VT_ATTN_DECODE_KV_SPLITS", 16) == 16 with the variable unset — that is default == default. It tests the helper's fallback, not the product, and would pass identically if the env var had never existed. That is a defect of ours and it is why three non-existent knobs look real from inside the repo. I am filing it separately.

The smaller items, unchanged from the earlier review

1. No issue, in any of the three required places. The spec header cites (#317), which is a merged PR rather than an issue; the roadmap has no entry; the PR body has none. grep -rn "GEMMA4-ROCM-KEEP" also matches nothing, so the row ID your branch name asserts does not exist yet.
2. The numbers are on the wrong surfaces. docs/BENCHMARKS.md and the append-only .agents/benchmark-record.md are both untouched. The latter exists precisely so nobody re-runs a closed lever — your five rejected levers are exactly what it is for. benchmark-record.md is append-only, so there is no merge-lock reason to avoid it.
3. docs/FEATURES.md:335 trades information for a number: the edit deletes Env VT_GEMMA4_*/VT_ATTN_*, seam test_gemma4_rocm_fp8_seams — the two facts that belong on a feature surface — for a bare absolute with no denominator.
4. Name the comparison stack and label it. "Vulkan Q8" does not say whose Vulkan. The prior entry for this same box labelled it llama.cpp Vulkan Q8 … separate stack, and docs/ROCM.md:300-303 says of this milestone: "The bar is vLLM, not llama.cpp." Your "does not claim that bar" framing is the right instinct and is one sentence from being fully admissible — there is a template at docs/BENCHMARKS.md:37 (the Muse Glimmer row) that names the absence, labels the secondary, and declares the vLLM axis an OPEN GAP.
5. docs/ENVIRONMENT.md:198 documents GEMM_M default 256, range 16..2048; the code says default 2048, range 16..8192 (gemma4_moe.cpp:1016-1021). Worth correcting in the same edit — your sentence reads as a lab override of a documented 256 when 2048 is already what the binary does.
6. Decode regime: 55.5 / 49.1 t/s with no concurrency, depth or batch size cannot be compared to anything. Your prefill depth curve is exactly right; decode just needs the same treatment.

Please keep the residual section as-is

The HIP cm1 339-spill vs ACO 0-spill contrast on the same silicon, with the explicit refusal to call it a named LLVM defect, is the most reusable thing in this PR. I would suggest an issue for it so it survives the spec being compacted later.

Nothing here needs new measurements — just the provenance question answered.