Skip to content

[Misc] Add per-kernel ISA resource delta table - #928

Open
Phil-amd wants to merge 1 commit into
mainfrom
phil/isa-resource-delta-table
Open

[Misc] Add per-kernel ISA resource delta table#928
Phil-amd wants to merge 1 commit into
mainfrom
phil/isa-resource-delta-table

Conversation

@Phil-amd

Copy link
Copy Markdown
Member

Summary

Adds scripts/isa_resource_table.py, which extracts per-kernel VGPR/SGPR/spill/scratch/ds_read counts from the 21_final_isa.s files of a FLYDSL_DUMP_IR tree and diffs two such trees.

Motivation

A green test suite says nothing about register pressure. A codegen change that alters address arithmetic can leave every test passing while quietly adding VGPRs or introducing spills in one kernel, and nothing in CI surfaces that today.

This came out of the #898 work, where it caught a real regression: an early version of that fix inflated fused_add_rmsnorm_kernel_0 from 9 to 27 VGPRs on gfx950 while the whole suite still passed. The tool is useful independently of that fix, so it is sent on its own.

Changes

  • scripts/isa_resource_table.py — summarize one dump tree (--json to save), or --diff before.json after.json to compare.
  • tests/unit/test_isa_resource_table.py — 13 cases pinning the assembly shape the parser depends on.

Two properties worth calling out, both learned the hard way:

  • Scoped per kernel. One ISA file can hold several kernels. Reading the first metadata match while counting instructions file-wide merges two kernels into one row and hides any regression in the second — 2 of 33 kernels in a typical run sit in such files. Register counts come from each kernel's own amdhsa.kernels entry; instruction counts from its body alone, bounded by whichever comes first of .Lfunc_endN or the next kernel's label.
  • Refuses to guess. The parser reads LLVM assembly text, which is not a stable interface. An unrecognized layout yields None rather than a plausible number, and --diff exits 2 when the comparison itself cannot be trusted: no kernels on either side, a kernel present on only one side, or an unparsed metric. A gate that cannot distinguish "no regressions" from "no data" manufactures false confidence.

Usage:

FLYDSL_DUMP_IR=1 FLYDSL_DUMP_DIR=/tmp/before python3 -m pytest <cases>
python3 scripts/isa_resource_table.py /tmp/before --json before.json
# apply change, repeat into after.json
python3 scripts/isa_resource_table.py --diff before.json after.json

ARCH=gfx950 COMPILE_ONLY=1 makes this work for an architecture you do not have hardware for, since the dump happens before kernel launch.

Performance (if applicable)

Not applicable — tooling only, no product code touched.

Testing

  • Unit tests added/updated — 13 cases, including the multi-kernel split and every untrustworthy-input path
  • Performance benchmarks run — not applicable
  • Tested on MI300X — run against real dump trees on gfx942 (33 kernels) and gfx950 (31 kernels); ruff and black --line-length 120 clean

Dependencies

  • No new third-party dependencies added

Breaking Changes

None. New script plus its tests; no existing file is modified.

@Phil-amd Phil-amd self-assigned this Jul 30, 2026
@Phil-amd
Phil-amd requested a review from coderfeli July 30, 2026 09:00
A passing test suite hides VGPR and spill regressions, so a codegen change
that alters address arithmetic needs its resource impact diffed per kernel.

Each entry is scoped to one kernel, since an ISA file can hold several, and
--diff exits non-zero when the comparison itself cannot be trusted: missing
kernels or unparsed metrics must not read as "no regressions".

Refs #898.
@Phil-amd
Phil-amd force-pushed the phil/isa-resource-delta-table branch from fcc0d7e to aa6a2d4 Compare July 31, 2026 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant