Skip to content

[Tools] Add architecture-general ISA resource diff tool #1015

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Phil-amd wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[Tools] Add architecture-general ISA resource diff tool #1015

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
221 changes: 221 additions & 0 deletions .claude/skills/isa-resource-diff/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,221 @@
---
name: isa-resource-diff
description: >
Detect per-kernel GPU resource regressions (VGPR, SGPR, register spills, scratch,
static LDS) by diffing the final ISA before and after a change, using
scripts/isa_resource_table.py. Compile-only: needs no GPU and no profiler run, so
it works on any target the compiler supports and runs in seconds. Use when asked
whether a change increased register pressure, caused spilling, or hurt occupancy,
when reviewing a kernel change for resource impact, or as a fast pre-check before
spending a profiling run.
Usage: /isa-resource-diff [<test-or-command>] [--arch <gfx>]
allowed-tools: Read Write Bash Grep Glob
---

# ISA Resource Diff

Compare per-kernel register, spill, scratch, and LDS usage between two builds to
catch resource regressions that functional tests do not surface.

## Pick the right skill first

| Question | Skill |
|---|---|
| Did my change increase registers / cause spills / grow LDS? | **this skill** (compile-only, seconds, no GPU) |
| *Why* is this kernel slow — which instructions stall, and on what? | `/kernel-trace-analysis` (needs a GPU run + rocprofv3 ATT trace) |
| Which commit made it slow? | `/bisect-perf-regression` (needs a runnable benchmark) |
| How do I collect a trace at all? | `/capture-kernel-trace` |

This skill measures **resources, not time**. A clean result here does not mean
performance is unchanged — it means register/LDS/spill pressure is unchanged.
A regression here is a strong, cheap signal that is usually worth acting on
before profiling, because spilling and occupancy cliffs dominate most kernel
slowdowns. See §7 of `docs/kernel_tuning_guide.md` for what to do about one.

## Arguments

| Argument | Required | Description |
|---|---|---|
| `<TEST-OR-COMMAND>` | No | What to run to produce dumps. Defaults to asking the user. Example: `pytest tests/kernels/test_softmax.py -q` |
| `--arch <gfx>` | No | Target for compile-only runs, e.g. `gfx950`. Omit to use local hardware |

If the user already has two dump directories or two JSON snapshots, skip to Step 3.

## Workflow

### Step 1 — Capture the "before" side

Check out or stash to the baseline state first, then:

```bash
FLYDSL_DUMP_IR=1 FLYDSL_DUMP_DIR=/tmp/isa-before FLYDSL_RUNTIME_ENABLE_CACHE=0 \
python3 -m pytest tests/kernels/test_softmax.py -q
```

`FLYDSL_RUNTIME_ENABLE_CACHE=0` is **required, not optional** — see Pitfalls.

For a target without local hardware, add `ARCH=<gfx> COMPILE_ONLY=1`:

```bash
ARCH=gfx950 COMPILE_ONLY=1 \
FLYDSL_DUMP_IR=1 FLYDSL_DUMP_DIR=/tmp/isa-before FLYDSL_RUNTIME_ENABLE_CACHE=0 \
python3 -m pytest tests/kernels/test_softmax.py -q
```

### Step 2 — Capture the "after" side

Apply the change, then rerun **the identical command** into a *fresh* directory
(`/tmp/isa-after`). Same test, same parameters, same arch, same cache setting.

### Step 3 — Diff

```bash
python3 scripts/isa_resource_table.py diff /tmp/isa-before /tmp/isa-after
```

The tool requires **Python 3.10+**. If `python3` is older it exits 2 with a clear
message; use `python3.10 scripts/isa_resource_table.py …` instead.

Both sides may independently be a dump directory or a `.json` snapshot, so a
baseline can be captured once and reused:

```bash
python3 scripts/isa_resource_table.py summarize /tmp/isa-before --json baseline.json
python3 scripts/isa_resource_table.py diff baseline.json /tmp/isa-after
```

## Reading the output

```
* = regression trigger; other columns are informational.
vgpr = total (arch+acc, LLVM's occupancy number); arch_vgpr/agpr are its split -- do not add them.
kernel *vgpr arch_vgpr agpr ... *lds_static_bytes lds_read ...
-------------------------------------------------------------------------------------------------
gemm::d128_fmha_fwd_kernel_0 942->960(+18) 942->960(+18) 0 ... 212992->229376(+16384) 12

compared 1 of 1 kernels; 0 unchanged; 1 changed; worsened: 2; improved: 0
RESULT: REGRESSION
```

Only changed and problematic kernels are printed. The last two stdout lines are

@jhinpan jhinpan Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non-blocking] Both stdout-shape claims are false exactly when there are problems -- the case an agent scripting against this contract most needs to get right. _render_problems prints its count of untrustworthy items to stdout, between the count line and the verdict:

$ isa_resource_table.py diff <two real gfx950 dump trees> 2>/dev/null | tail -3
compared 2 of 6 kernels; 2 unchanged; 0 changed; worsened: 0; improved: 0
4 kernel(s)/metric(s) not trustworthy (see stderr)
RESULT: NOT TRUSTWORTHY

The last two stdout lines are that count and the verdict, not "a count line and a RESULT: verdict". Line 147's "-q/--quiet prints only the verdict line" fails the same way, on the same run:

$ isa_resource_table.py diff <same two trees> -q 2>/dev/null
4 kernel(s)/metric(s) not trustworthy (see stderr)
RESULT: NOT TRUSTWORTHY

Two ways to close it. Narrow both sentences to "the last stdout line is always the RESULT: verdict" -- true unconditionally, and all a script needs. Or route that one line to stderr with the rest of _render_problems, which would make both sentences true as written and match "Warnings on stderr never change the exit code" in Pitfalls.

always a count line and a `RESULT:` verdict that matches the exit code exactly.

**Columns marked `*` are regression triggers**; the rest are context.

| Column | Trigger | Read from | What it is |
|---|---|---|---|
| `vgpr` | yes | `.vgpr_count` metadata | Total VGPRs, arch + accumulator — LLVM's own occupancy number |
| `arch_vgpr` | no | `.set` symbol `num_vgpr` | The arch half of that total |
| `agpr` | no | `.set` symbol `num_agpr` | The accumulator half of that total |

@jhinpan jhinpan Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[non-blocking] Same boundary as the blocking vgpr finding in round 4. "The accumulator half of that total" holds only where .vgpr_count is the arch+accumulator sum, which LLVM makes it only on gfx90a+; on gfx908 it writes max(arch, agpr), so agpr is not a component of vgpr at all there.

That makes the "Do not add arch_vgpr and agpr to vgpr" note below read as a stronger guarantee than the metadata format gives, and it is the sentence a reader will use to decide that a moved AGPR count needs no attention.

docs/kernel_tuning_guide.md in this same PR already scopes it correctly ("targets with a unified register file (gfx90a and later MFMA-capable parts)"). Carrying that same qualifier into this table -- and into the vgpr row above it -- would make the two documents agree. Whatever the code ends up doing about the trigger, this table should describe the format, not the gfx90a+ special case.

| `sgpr` | yes | `.sgpr_count` metadata | SGPRs including the fixed extras (VCC, XNACK, FLAT_SCRATCH) |
| `numbered_sgpr` | no | `.set` symbol `numbered_sgpr` | SGPRs without those extras; tells a real increase from VCC becoming live |
| `vgpr_spill` | yes | `.vgpr_spill_count` metadata | VGPRs the allocator spilled |
| `sgpr_spill` | yes | `.sgpr_spill_count` metadata | SGPRs the allocator spilled |
| `scratch_bytes` | yes | `.private_segment_fixed_size` metadata | Private segment per work-item; exact on every target |
| `lds_static_bytes` | yes | `.group_segment_fixed_size` metadata | Statically allocated LDS per work-group |
| `lds_read` / `lds_write` | no | instruction count | `ds_read`/`ds_load` and `ds_write`/`ds_store` sites |
| `scratch_store` / `scratch_load` | no | instruction count | `scratch_*` sites; `n/a` where spilling goes through `buffer_*` |
| `matrix_ops` | no | instruction count | MFMA / WMMA sites |

Three things are easy to misread:

- **Do not add `arch_vgpr` and `agpr` to `vgpr`.** `vgpr` is already the total
(arch + accumulator) and is the only VGPR-family trigger. The other two are its
split, shown so you can tell *which half* moved. Moving accumulators into AGPRs
— which the tuning guide recommends — deliberately does not count as a regression.
- **`n/a` is not `0`.** It means the quantity does not exist on this target, e.g.
`scratch_store`/`scratch_load` on a target that spills through `buffer_*`. Use
`scratch_bytes`, which is exact everywhere.
- **`?` means unparsed** — the tool could not read something it reports. Any `?`
forces exit 2. Never treat it as unchanged.

### Exit codes

| Code | stdout | Meaning | What an agent should do |
|---|---|---|---|
| `0` | `RESULT: OK` | Everything comparable, no trigger increased | Proceed |
| `1` | `RESULT: REGRESSION` | Everything comparable, a trigger increased | Investigate — this is a real finding |
| `2` | `RESULT: NOT TRUSTWORTHY` | The tool cannot answer | **Fix the inputs and rerun. Do not report "no regression"** |

Exit `1` is a claim about the code under test; exit `2` is a claim about the
tool's own confidence. Everything that would leave the answer partial reports `2`
and never `1`: a crash, an empty dump directory, a dump file that does not parse
or does not decode, a metadata entry with no kernel identity or a duplicated one,
a negative resource count, a kernel present on only one side, and a target that
differs — or that the tool cannot name — on either side.

For scripting, `-q/--quiet` prints only the verdict line, and `--json PATH`
writes the full comparison as machine-readable JSON:

```bash
python3 scripts/isa_resource_table.py diff /tmp/isa-before /tmp/isa-after -q --json report.json
case $? in
0) echo "no resource regression" ;;
1) echo "REGRESSION — see report.json" ;;
*) echo "inconclusive — inputs are bad, do not claim a clean result" ;;
esac
```

## Acting on a regression

Map the column that moved to a cause, then follow `docs/kernel_tuning_guide.md`:

| Column increased | Usual cause |
|---|---|
| `vgpr` | More live values; larger tiles or deeper pipelining/unrolling |
| `vgpr_spill` / `sgpr_spill` / `scratch_bytes` | Register pressure crossed the budget — normally the most damaging of these signals |
| `lds_static_bytes` | Bigger shared tiles or added double buffering; may cross an occupancy step |
| `sgpr` alone, by 2, with `numbered_sgpr` flat | Usually just VCC becoming live — rarely meaningful |

If a resource regression is confirmed but the kernel is not actually slower,
say so rather than "fixing" it: these are proxies for occupancy, not timings.
Confirm with `/kernel-trace-analysis` before reworking a kernel.

## Pitfalls

These silently produce a *confident wrong answer* if ignored:

- **Dump directories are keyed by kernel name only, with no specialization key.**
Every JIT specialization of one kernel writes to the same directory and
overwrites the previous one, so a parametrized test leaves only its last
variant. Diff one shape at a time when the answer must be exact.
- **A cache hit produces no dump at all.** Hence `FLYDSL_RUNTIME_ENABLE_CACHE=0`
on both sides. Without it a kernel can silently vanish from one side, which
the tool reports as `ONLY IN BEFORE` and exit 2.
- **`lds_static_bytes` is static LDS only.** A kernel using
`SharedAllocator(static=False)` reports `0` no matter how much LDS it takes at
dispatch; the tool prints a `dynamic LDS in use` note for it. LDS regressions
in those kernels are invisible here.
- **The stage number in `NN_final_isa.s` varies between runs.** Nothing cleans the
dump directory, so reusing one can leave two files side by side. The tool uses
the highest-numbered one and warns; prefer a fresh directory per run.
- **Diffs across targets are refused** (exit 2). Register files and LDS banking
differ between architectures, and `xnack`/`sramecc` change code generation
within one, so both sides must report the same processor *and* the same target
features. A target the tool cannot name is refused for the same reason. The
triple's environment field is normalized, so `amdgcn-amd-amdhsa--gfx942` and
`amdgcn-amd-amdhsa-unknown-gfx942` are the same target.
- **Warnings on stderr never change the exit code.** They describe the input tree
and are worth reading before trusting a clean result.

## Verifying the tool itself

`tests/unit/test_isa_resource_table.py` is backend-agnostic and needs no build:

```bash
python3 -m pytest tests/unit/test_isa_resource_table.py -q
```

It generates its ISA input in `make_isa()` rather than checking dumps in, so the
test file states exactly which parts of LLVM's output the parser relies on. The
shapes cover both sides of each axis that has already broken it once: whether the
target emits `.agpr_count`, which LDS mnemonic spelling it uses, and whether the
target ID spells the triple's environment as empty or as `unknown`. The rest of
the file pins the fail-closed verdicts: a mismatched processor, a mismatched
feature set, an unnameable target, an unreadable or undecodable dump file, a
kernel entry with no identity or a duplicated one, and a negative count all have
to reach exit 2.

If the tool reports `?` on a dump that looks healthy, LLVM's assembly format has
probably drifted — update `make_isa()` to match the new shape rather than
loosening the parser, since the failure is deliberately loud.
5 changes: 5 additions & 0 deletions .claude/skills/kernel-trace-analysis/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ an optimization plan.
All analysis is done programmatically via `hotspot_analyzer.py` + `code.json`.
Do **not** use GUI tools.

> If the question is only "did my change increase register pressure, spills, or
> LDS?", use `/isa-resource-diff` first — it is compile-only, needs no GPU or
> profiler run, and answers in seconds. Come here when you need to know *why* a
> kernel is slow rather than *what resources it uses*.

## Arguments

| Argument | Description |
Expand Down
23 changes: 23 additions & 0 deletions docs/kernel_tuning_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,29 @@ WHERE ks.KernelName LIKE '%target_kernel%' LIMIT 5;
the binding limiter, so you know whether to cut VGPR, shrink LDS per block, or
reduce SGPR pressure.

For a lightweight before/after check without collecting a profiler trace, dump
the final ISA of each build and diff the per-kernel **total VGPR (with its
arch/accumulator split), SGPR, register spills, scratch bytes, and static LDS**
with `scripts/isa_resource_table.py`. It is compile-only, so it needs no GPU:

```bash
FLYDSL_DUMP_IR=1 FLYDSL_DUMP_DIR=/tmp/isa-before FLYDSL_RUNTIME_ENABLE_CACHE=0 \
python3 -m pytest tests/kernels/test_softmax.py -q # repeat for /tmp/isa-after
python3 scripts/isa_resource_table.py diff /tmp/isa-before /tmp/isa-after
```

It exits `0` for no regression, `1` for one, and `2` when it cannot produce a
trustworthy answer — a crash never reports as `1`. The table marks regression
triggers with `*` and prints a legend above itself; `/isa-resource-diff` carries
the full column reference. Set `FLYDSL_RUNTIME_ENABLE_CACHE=0` on both runs: a
cache hit emits no dump at all, which would silently drop a kernel from one side.

Because arch VGPRs and accumulator VGPRs share one register budget on targets
with a unified register file (gfx90a and later MFMA-capable parts), that
tool treats the **total** as the regression signal and reports `arch_vgpr` and
`agpr` alongside it for information only. Moving accumulators into AGPRs, as
recommended below, therefore does **not** register as a resource regression.

**Do not** use `maxnreg` to force `accum_vgpr=0` — it spills MFMA results through
arch_vgpr via `v_accvgpr_read` (measured ~4.5× regression).

Expand Down
1 change: 1 addition & 0 deletions docs/testing_benchmarking_guide.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ bash scripts/dumpir.sh
| `scripts/run_tests.sh` | Full test runner (pytest + examples + FileCheck) |
| `scripts/run_benchmark.sh` | Benchmark harness with configurable shapes |
| `scripts/dumpir.sh` | IR dump helper script |
| `scripts/isa_resource_table.py` | Diff per-kernel ISA resource usage (compile-only) |
| `tests/conftest.py` | Pytest fixtures (MLIR context, module, insert point) |
| `tests/test_common.py` | `perftest()`, `checkAllclose()`, `verify_output()` |
| `tests/utils.py` | `pertoken_quant()`, `shuffle_weight()` |
Expand Down
Loading
Loading