feat(rocke): shape-generic (AOT) Winograd forward transforms for gfx1250 by jakpiase · Pull Request #14791 · ROCm/rocm-libraries

jakpiase · 2026-10-30T21:19:08Z

Motivation

The Winograd F(m,3) forward pipeline on gfx1250 builds its transform kernels (K1 filter, K2a prescale max, K2 input, K4 output) per problem: the extents are baked in, so every convolution shape needs its own binaries and the pipeline cannot be served from an AOT kernel cache next to the direct and implicit-GEMM instances. The K3 batched GEMMs already take their extents at run time.

ISSUE ID : AICK-2884

Technical Details

Shape-generic transforms: kernels/common/conv_winograd_fwd_rt.py

The same K1 / K2a / K2 / K4 stages, workspace layouts and arithmetic as conv_winograd_fwd. Every problem extent is a kernel argument, so one binary serves every problem its knobs fit.
Divisions by runtime extents are a multiply-high by a host-computed magic number (magic_div), exact for 31-bit indices.
One spec per stage: RtFilterSpec, RtPrescaleSpec, RtInputSpec, RtOutputSpec. The compile-time knobs are the tile, the dtypes, the channel vector width, the workgroup size, the K2 register run or LDS region, and the prescale. Every field is defaulted.
K2 keeps the register path, with a fused prescale over a runtime number of channel lanes (up to the workgroup size), and the LDS region. By default the fused K2 runs on the smallest block (256, 512 or 1024) that holds all lanes. Wider C, and tiles over 36 bins past 512 lanes, run the split prescale (K2a).
stage_launch returns a stage's grid, block and scalar kernargs. aot_stage_specs() is the AOT grid. default_stage_specs(problem, tile, dtype) is the default per-problem pick: the JIT pipeline's tuned knobs, narrowed to the problem's channel extents.

Both engines

C++ port in cpp/instances/common/conv_winograd_fwd_rt.cpp (instance_conv_winograd_fwd_rt.h), byte-identical IR.
The shared Winograd math helpers (bt_1d, at_1d, filter_matrix_int, the prescale bit helpers) move from conv_winograd_fwd.cpp into rocke_wino. There is no IR change to the existing families.
Parity pair conv_winograd_fwd_rt_emit.{py,c}, 50 configs:
- every stage;
- all tiles, fp16 and bf16;
- fused and split prescale;
- the LDS path;
- vec 1/2/4/8;
- gfx950;
- 16 specs both engines reject.
A representative IR golden shard conv_winograd_fwd_rt.

Dispatch

New plan kind unfused2d_rt in dispatch/conv_winograd_plan.py. Each transform step is named by its binary's kernel name (its AOT identity) and carries the problem in its kernargs.
New opt-in candidate conv_winograd_fwd_aot (algorithm="winograd_aot"). It runs the selector's 3x3 plans on the shape-generic binaries. algorithm="auto" never picks it, and the existing conv_winograd_fwd route is unchanged.

Benchmark

benchmarks/common/benchmark_winograd_aot.py:
- --compile-all builds the grid into a cache.
- --run-from-cache assembles each case's pipeline from it. For every tile it picks each stage's binary by timing, verifies the assembled pipeline against the reference convolution, and times the chain (--timing staged|block).
- It takes the same inputs and writes the same rows as the other AOT conv sweeps: --miopen-file / --miopen-cmd / shape flags, --dtype / --direction, --results (case_results rows, engine winograd), --skip-measured, and the standard Best: line. Cases the winograd_aot route refuses are skipped with the reason.
benchmark_conv_compare.py runs it as a third engine after direct-conv and implicit-GEMM. It runs by default on a --run-from-cache comparison whose directory also holds the Winograd cache, and --skip-winograd turns it off. --results merges the three engines on one row (winograd_* columns; the winner and speedup are taken over every engine that measured the case).
run_conv_shards.py --script winograd shards the Winograd sweep.

Test Plan

Byte-identity gate (tools/check_byte_identity.py --only winograd) at llvm20, llvm22, llvm23 and llvm24.
tests/test_rocke_ci_static.py (IR parity coverage and golden).
library/tests/test_conv_winograd_fwd_rt_cpu.py:
- magic_div exactness;
- spec refusals;
- AOT grid builds;
- the default pick is in the grid;
- stage_launch kernargs.
library/tests/dispatch/test_conv_winograd_aot_dispatch.py: route selection, launch-plan steps and kernargs, batch split, refusals.
library/tests/test_run_conv_shards.py: the three-engine merge, CSV and summary; compare forwarding the Winograd args and skipping it (no cache, JIT, non-fwd, --skip-winograd); its exit status; the AOT script's case reading and refusals.
On gfx1250, by hand: a three-engine benchmark_conv_compare.py --run-from-cache --verify --results run; a mixed case file (-F 0, 1x1, stride 2); block timing; --skip-measured resume; run_conv_shards.py --script winograd.
Existing test_conv_winograd_dispatch.py, test_grouped_conv_dispatch.py, test_library_layering.py, test_builder_signature_contract.py.
On gfx1250, library/tests/test_conv_winograd_fwd_rt_gpu.py compares against the conv reference (max|out - ref| / max(max|ref|, 1) < 5e-2) with poisoned workspaces. It covers:
- every tile, fp16 and bf16;
- bf16 wide C (C = 256, 2048, 4096): fused prescale at blocks 256 / 512 / 1024 and the split;
- dilation, groups, odd extents;
- batch split;
- forced register-run, LDS-region, vec-1 and split-prescale binaries.
pre-commit run on the changed files.

Test Result

Byte-identity gate: GREEN for all five Winograd families at every flavor.
conv_winograd_fwd_rt: 34 configs identical, 16 rejected by both engines.
CPU tests: 225 passed. Static IR parity and golden: 8 passed. After the compare integration: test_run_conv_shards.py 88 passed; together with the Winograd CPU, dispatch, layering and signature tests, 182 passed.
gfx1250 compare / sweep / shard runs: all exit 0, every reported pipeline verified.
GPU numeric test on gfx1250: 17 passed.
pre-commit: clean.

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests.

The Winograd F(m,3) transform kernels (K1 filter, K2a prescale max, K2 input, K4 output) bake the problem extents in, so every convolution shape needs its own binaries and the pipeline cannot be served from an AOT cache. conv_winograd_fwd_rt builds the same stages, workspace layouts and arithmetic with every extent as a kernel argument (divisions by runtime extents through a host-computed magic number), so one binary serves every problem its knobs fit. The K3 batched GEMMs already take their extents at run time. - kernels/common/conv_winograd_fwd_rt.py: per-stage specs (RtFilterSpec, RtPrescaleSpec, RtInputSpec, RtOutputSpec), stage_launch (grid, block and scalar kernargs), the AOT grid (aot_stage_specs) and the default per-problem pick (default_stage_specs). K2 keeps the register path (with a fused prescale over a runtime lane count) and the LDS region. - C++ engine port with byte-identical IR; the shared Winograd math helpers move into rocke_wino (no IR change to the existing families). - conv_winograd_fwd_rt_emit.{py,c} parity pair and a representative IR golden shard. - Dispatch: plan kind unfused2d_rt and the opt-in candidate conv_winograd_fwd_aot (algorithm="winograd_aot") over the selector's 3x3 plans; algorithm="auto" never picks it. - benchmarks/common/benchmark_winograd_aot.py: --compile-all builds the grid into a cache, --run-from-cache assembles each case's pipeline from it, picking each stage's binary by timing and verifying the pipeline. - Tests: CPU (kernels, dispatch) and on-GPU numerics; docs and support matrix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

therock-pr-bot · 2026-10-30T21:24:52Z

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

therock-pr-bot · 2026-10-30T21:24:55Z

🎉 All checks passed! This PR is ready for review.

Copilot

🟡 Changes recommended

Validation gaps allow unsupported geometry and non-power-of-two LDS lane counts, leading to incompatible kernel launches and missing or duplicate stores.

6 open issues

What changed in this PR

Adds shape-generic Winograd forward transforms for gfx1250, allowing compiled binaries to be reused across convolution shapes through an opt-in AOT dispatch route while preserving the existing route.

Changes:

Introduces runtime-extent transform specs, launch arguments and AOT binary configurations.
Adds winograd_aot dispatch and a cache compilation, tuning and verification benchmark.
Shares C++ transform helpers and adds parity, golden, CPU and GPU coverage with supporting documentation.

Paths are relative to dnn-providers/hip-kernel-provider/rocke/.

File	Description
`platform/tests/instances/rocke_ir_parity_harness.py`	Registers representative runtime-transform IR cases.
`platform/tests/golden/rocke_representative_ir_sha256/conv_winograd_fwd_rt.json`	Adds LLVM-flavor golden hashes.
`platform/python/rocke/instances/SUPPORT_MATRIX.md`	Documents AOT Winograd availability.
`platform/dsl_docs/instances/index.md`	Indexes the new transform specs.
`platform/dsl_docs/instances/convolution.md`	Explains runtime transforms and dispatch.
`platform/cpp/instances/common/conv_winograd_fwd.cpp`	Delegates transform math to shared helpers.
`platform/cpp/instances/common/conv_winograd_common.cpp`	Houses reusable transform and prescale helpers.
`platform/cpp/include/rocke/instance_conv_winograd_internal.h`	Declares shared C++ helpers.
`platform/cpp/include/rocke/instance_conv_winograd_fwd_rt.h`	Defines the runtime-transform C API.
`library/tests/test_conv_winograd_fwd_rt_gpu.py`	Adds end-to-end numeric and binary-reuse tests.
`library/tests/test_conv_winograd_fwd_rt_cpu.py`	Tests division, specs, AOT configurations and launches.
`library/tests/test_builder_signature_contract.py`	Registers defaulted runtime-transform specs.
`library/tests/parity/conv_winograd_fwd_rt_emit.py`	Adds Python parity configurations and emission.
`library/tests/parity/conv_winograd_fwd_rt_emit.c`	Adds matching C-side parity emission.
`library/tests/dispatch/test_conv_winograd_aot_dispatch.py`	Tests routing, launch arguments and batch splitting.
`library/kernels/common/conv_winograd_select.py`	Labels runtime-transform plans.
`library/kernels/common/conv_winograd_fwd_rt.py`	Implements shape-generic transforms and launch geometry.
`library/dispatch/grouped_convolution.py`	Registers the opt-in AOT candidate.
`library/dispatch/conv_winograd_plan.py`	Assembles runtime-transform launch pipelines.
`library/benchmarks/common/benchmark_winograd_aot.py`	Compiles caches and benchmarks assembled pipelines.

_{Review effort: Balanced}

Review follow-ups for the shape-generic Winograd forward transforms: - Both engines refuse an LDS lane count that is not a positive power of two (the lane mapping is a shift and mask), an LDS region of zero width, a filter_rows below 1 and a K2 / K4 block above 1024 threads. - stage_launch runs RtGeometry.check itself, so a non-3x3, strided or oversized problem is refused there and not only by the callers. - The Winograd launch plans refuse a batch split that is not a positive integer instead of normalizing it. - Six new rejected parity configs; the new CPU, dispatch and GPU tests are in the installed CTest lists. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

cgmillette · 2026-10-31T02:01:55Z

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 First-round: wide-C fused K2 spills, untested

Prior item	Status
Copilot (10-30) ×6: LDS lanes; `stage_launch` geometry; batch split; `filter_rows`; block ≤ 1024; CMake lists	🟢 FIXED `2b5b2730` • both engines refuse: parity cfgs 44–49 (ran) • refusal tests: CPU 55 ✓, dispatch 14 ✓, also in CI (ran) • all 3 test files listed, GPU at `CMakeLists.txt:869` (read)
bartekxk (10-30) `benchmark_winograd_aot.py:201`: in `benchmark_conv_compare`?	🟡 OPEN • no: it runs only the direct and implicit-GEMM scripts (read) • the JIT Winograd benchmark isn't in it either • jakpiase to reply
Bots / CI	🟢 Required ✓ • fails: HipDNN Windows Superbuild + Summary — develop's census configure error, fix #14801 (read) • 41 ✓, incl. Linux and Windows hipkernelprovider shards • merge: clean vs today's `809c17de2f` (ran)
Our checks	🟡 1 new spill • merge vs `2d675fdefd`: clean; build + ctest 35/35; rules 1–3 clean (ran) • byte identity GREEN ×3, 4 families; goldens pass (ran) • differential: 0 changed; new 309 = develop's 275 + these 34 (ran) • tests 78 ✓ / 14 skip (GPU: gfx1250 + torch); compile: cfg 16 spills, N1 (ran)

#	Sev	file:line	Finding
N1	🟡	`conv_winograd_fwd_rt.py:1459`, `:1511`; `test_conv_winograd_fwd_rt_gpu.py:139`	Wide-C fused prescale K2: block-1024 binaries spill, no GPU case • impact: 13 of 356 AOT binaries spill, all fused block 1024; 6 are defaults, e.g. bf16 F4 C=2048 (ran) • impact: GPU cases stop at C=128: multi-row max (C/vec > 32) never runs (read) • fix: fused up to block 512 (defaults don't spill, ran), split beyond; add a bf16 C ≥ 256 case

Utility: 🟢 Positive — AOT-cacheable 3x3 Winograd transforms behind an opt-in route; wide-C untested on GPU.

Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

First-round review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: merge with develop, build + ctest, byte identity, goldens, PR tests, gfx1250 compile and AOT-grid spill sweep, CI at head. Flags whether a closer look is needed; not an approval or rejection.

The default shape-generic K2 with the fused prescale ran at block 1024 for anything over 256 channel lanes. A 1024-thread workgroup caps a wave at 128 VGPRs, so the wider binaries spilled, and at 512 lanes the block-1024 K2 was slower than the block-512 one the JIT pipeline uses (block = max(block, lanes)). - The AOT grid adds fused K2 binaries at block 512. - The default is the smallest block of 256 / 512 / 1024 that holds all lanes; tiles over 36 bins (F(6x4), F(4x6)) take the split prescale past 512 lanes, where their block-1024 K2 spills most and the split is faster. - CPU test of the block choice; GPU cases for bf16 C = 256, 2048 and 4096 (the fused max over more than one wave, blocks 512 / 1024, and the split). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jakpiase · 2026-10-31T11:19:08Z

Re N1 (wide-C fused K2 spills): fixed in 0f78ac0.

The AOT grid adds fused K2 binaries at block 512. The default is now the smallest block of 256 / 512 / 1024 that holds all channel lanes, the same as the JIT pipeline (block = max(block, lanes)). On gfx1250 the block-512 K2 was faster than both the block-1024 one and the split prescale at 512 lanes.
At 1024 lanes, the block-1024 fused K2 was still faster than the split on F4 / F(4x2) / F(2x4) and tied on F(6x2) / F(2x6), so it stays. F(6x4) / F(4x6) (over 36 bins) spill the most and now take the split past 512 lanes.
New tests: a CPU test of the block choice, and GPU cases for bf16 C = 256 (fused max over more than one wave), C = 2048 (block 512) and C = 4096 (block 1024 and split). The gfx1250 GPU test passes (17).

cgmillette · 2026-10-31T13:55:34Z

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🔴 Follow-up: llvm24 alias breaks goldens

Prior item	Status
Copilot (10-30) ×6: LDS lanes; `stage_launch` geometry; batch split; `filter_rows`; block ≤ 1024; CMake lists	🟢 FIXED `2b5b2730` • fix commit unchanged; new commit leaves the validators alone (ran: range-diff) • refusal tests ✓ on the merge: CPU 67, dispatch 14 (ran); same in CI (read)
bartekxk (10-30) `benchmark_winograd_aot.py:201`: in `benchmark_conv_compare`?	🟡 DEFERRED • jakpiase: no; both Winograd benchmarks go to a follow-up PR • fair: the route is opt-in, JIT Winograd isn't wired in either • thread open: bartekxk to agree and resolve
Our 10-31 N1 🟡: wide-C fused K2 spills, no GPU case	🟢 FIXED `0f78ac00` • block-512 binaries; default = smallest block holding C; spilling defaults 6 → 4, 3 reachable (ran: 384-spec sweep) • left: bf16 F4, F(6x2), F(2x6) at 1024 lanes; author: faster than or tied with the split • GPU cases C = 256 / 2048 / 4096: 17 ✓ on gfx1250 (author-reported; CI skips)
Bots / CI	🟡 Running • fails: gfx94X hipkernelprovider ×2 — `test_ir_cases_match_golden_sha256` 60 s timeout, develop's 14733 (read) • running: Windows gfx110X hipkernelprovider ×2; 39 ✓ • merge: clean; CI merged develop `d5c3ba8e85`, before 14692, so N1 can't show there (read)
Our checks	🔴 Golden fails on the merge • merge vs `37e8aeb87c`: clean; build + ctest 34 ✓ / 1 skip; rules 1–3 clean (ran) • byte identity GREEN ×4 flavors (llvm24 by hand); differential +34 new per flavor, 0 changed (ran) • golden: FAIL at load ×4 → N1; tests 90 ✓ / 17 skip (GPU); compile: cfg 16 spills, no longer a default (ran)

#	Sev	file:line	Finding
N1	🔴	`golden/rocke_representative_ir_sha256/conv_winograd_fwd_rt.json:3`	New shard keeps the llvm24 → llvm23 alias; develop's shards dropped it (14692) • impact: on the merge, `rocke_golden_static` and `rocke_pytest` fail at golden load, all 4 flavors (ran) • fix: merge develop, then from `platform/`: `python tests/instances/rocke_ir_parity_harness.py --write tests/golden/rocke_representative_ir_sha256 --only conv_winograd_fwd_rt` • on a copy: alias gone, 6 llvm24 cases added, llvm20–23 unchanged; only develop's 14815 drift left (ran)

Utility: 🟢 Positive — AOT-cacheable 3x3 Winograd transforms behind an opt-in route; wide-C spills trimmed.

Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: range-diff since our review, merge with today's develop, build + ctest, byte identity ×4 flavors, golden gate and a scratch re-record, differential golden, PR tests, AOT-grid spill sweep with default picks, CI logs at head. Flags whether a closer look is needed; not an approval or rejection.

…ke/gfx1250-winograd-aot

develop now records LLVM 24 as its own golden flavor instead of an alias of LLVM 23. Re-record the conv_winograd_fwd_rt shard after merging develop: drop the llvm24 -> llvm23 alias and add the six llvm24 cases. The llvm20/22/23 hashes are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jakpiase · 2026-11-01T16:53:04Z

Re N1 (llvm24 alias in the new golden shard): fixed in 7fe0c22. I merged develop (ff9284d) and re-recorded the conv_winograd_fwd_rt shard with rocke_ir_parity_harness.py --write ... --only conv_winograd_fwd_rt: the alias is gone, the six llvm24 cases are added, and the llvm20/22/23 hashes are unchanged. The installed golden gate passes on all four flavors, and byte identity is GREEN at llvm20/22/23/24.

cgmillette · 2026-11-01T18:01:05Z

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 Follow-up: alias fixed, CI running

Prior item	Status
Copilot (10-30) ×6: LDS lanes; `stage_launch` geometry; batch split; `filter_rows`; block ≤ 1024; CMake lists	🟢 FIXED `2b5b2730` • fix unchanged at head; new commits leave the code alone (ran) • refusal tests on the merge: CPU 67 ✓, dispatch 14 ✓ (ran) • all six threads resolved
bartekxk (10-30) `benchmark_winograd_aot.py:201`: in `benchmark_conv_compare`?	🟡 DEFERRED • jakpiase: both Winograd benchmarks go to a follow-up PR • fair: the route is opt-in • thread resolved by jakpiase; no reply from bartekxk yet
Our 10-31 N1 🟡: wide-C fused K2 spills, no GPU case	🟢 FIXED `0f78ac00` • code unchanged since our last review (ran) • compile: only cfg 16 spills, not a default pick (ran) • gfx1250 GPU cases author-reported; CI skips them (no torch)
Our 10-31 follow-up N1 🔴: new shard keeps the llvm24 alias	🟢 FIXED `7fe0c229` • alias gone; 6 llvm24 cases; llvm20–23 hashes unchanged (ran) • a re-record on the merge reproduces the shard exactly (ran) • golden PASS ×4 flavors beside develop's 35 shards (ran)
Bots / CI	🟡 Running • running: gfx94X hipkernelprovider ×2, hipdnn ×2; Windows gfx110X math-libs build • 31 ✓, none failing • merge: clean; CI's merge includes 14692, so it checks the shard (read)
Our checks	🟢 Clean • merge vs `fce4a134a1`: clean; build + ctest 34 ✓ / 1 skip (ran) • byte identity GREEN ×4 flavors; golden PASS ×4; differential 0 changed (ran) • tests 90 ✓ / 17 skip (GPU); compile: cfg 16 spill only; rules 1–3 clean (ran)

Net-new: none.

Utility: 🟢 Positive — AOT-cacheable 3x3 Winograd transforms behind an opt-in route; LLVM 24 golden added.

Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: delta since our review (per-file patches, merge resolution), merge with today's develop, build + ctest, byte identity ×4 flavors, golden gate and a scratch re-record, differential golden, PR tests, compile, CI at head. Flags whether a closer look is needed; not an approval or rejection.

benchmark_conv_compare.py now runs benchmark_winograd_aot.py as a third engine beside the direct-conv and implicit-GEMM sweeps. It runs by default on a --run-from-cache comparison whose directory holds the Winograd cache, and --skip-winograd leaves it out. A JIT comparison, a non-fwd --direction or a missing cache skip it with an info line. - case_results: "winograd" is an engine, with its own merged and CSV columns. The winner is the fastest engine that measured the case, and the speedup is over the next best. - benchmark_winograd_aot.py reads the compare inputs: --miopen-cmd, the shape flags, and --dtype / --direction overrides. It skips the cases the winograd_aot route refuses before it loads the cache. It times the assembled pipeline with --timing staged|block. --results writes case_results rows, --skip-measured resumes (it replaces --resume), and it prints the standard Best: TFLOPS line. A case with no fitting pipeline exits 0; one whose pipelines all fail verification exits 1. - run_conv_shards.py --script winograd shards the sweep. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

jakpiase · 2026-11-01T18:45:03Z

@cgmillette thanks for the follow-up. Agreed: the llvm24 alias fix (7fe0c22) is in, and there were no net-new findings to address.

@bartekxk your benchmark_conv_compare question was marked as deferred in the review table; I have now implemented it in this PR instead of a follow-up (01cf588, details in your thread):

benchmark_conv_compare.py runs benchmark_winograd_aot.py as a third engine after direct-conv and implicit-GEMM. It runs by default on a --run-from-cache comparison when the cache directory also holds the Winograd cache, on 3x3 stride-1 forward cases. --skip-winograd turns it off. Without that cache, on a JIT comparison, or with a non-fwd --direction, it is skipped with an info line.
case_results: winograd is a third engine with its own merged/CSV columns. The winner and speedup are taken over every engine that measured the case (speedup = next best over the winner). Direct-vs-implicit output is unchanged.
benchmark_winograd_aot.py takes the same inputs as the other sweeps (--miopen-file/--miopen-cmd, shape flags, --dtype/--direction), times the assembled pipeline with --timing staged|block, writes case_results rows with --results, resumes with --skip-measured (replacing its old --resume), and prints the standard Best: line. It skips cases the winograd_aot route refuses before loading the cache. A case with no fitting pipeline exits 0; a case whose pipelines all fail verification exits 1.
run_conv_shards.py --script winograd shards the Winograd sweep on its own.

Testing: new host-side tests in test_run_conv_shards.py (88 passed there; 182 with the Winograd CPU, dispatch, layering and signature tests), ci_static 8 passed, black clean. On gfx1250 I ran by hand a three-engine compare with --verify --results, a mixed case file (-F 0, 1x1, stride 2), block timing, --skip-measured resume, and a sharded run; all exited 0 and every reported pipeline verified. The PR description is updated.

cgmillette · 2026-11-01T19:50:38Z

Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 Follow-up: compare integration verified, CI running

Prior item	Status
Copilot (10-30) ×6: LDS lanes; `stage_launch` geometry; batch split; `filter_rows`; block ≤ 1024; CMake lists	🟢 FIXED `2b5b2730` • `01cf5889` leaves this code untouched (ran) • refusal tests on the merge: CPU 67 ✓, dispatch 14 ✓ (ran) • all six threads resolved
bartekxk (10-30) `benchmark_winograd_aot.py:214`: in `benchmark_conv_compare`?	🟢 FIXED `01cf5889` • third engine in compare; merge, resume, shards OK (ran, GPU stubbed) • 23 new tests ✓, also in the installed layout (ran) • thread resolved by jakpiase; no reply from bartekxk yet
Our 10-31 N1 🟡: wide-C fused K2 spills, no GPU case	🟢 FIXED `0f78ac00` • kernel code unchanged since our last review (ran) • compile: only cfg 16 spills, not a default pick (ran) • gfx1250 GPU cases author-reported; CI skips them (no torch)
Our 10-31 follow-up N1 🔴: new shard keeps the llvm24 alias	🟢 FIXED `7fe0c229` • shard unchanged since; alias check passes on the merge (ran) • golden PASS ×4 flavors beside develop's shards (ran)
Bots / CI	🟡 Running • running: gfx94X hipkernelprovider ×2; Windows gfx110X math-libs build • 33 ✓, none failing; 1 cancelled: PR Bot, superseded by a ✓ run • merge: clean; CI merges the same develop `fce4a134` (read)
Our checks	🟢 Clean • merge vs `fce4a134`: clean; build + ctest 34 ✓ / 1 skip (ran) • byte identity GREEN ×4 flavors; golden PASS ×4; differential 0 changed (ran) • tests 178 ✓ / 17 skip (+88: `test_run_conv_shards.py`); compile unchanged; rules 1–3 clean (ran)

Net-new: none.

Utility: 🟢 Positive — opt-in AOT Winograd for gfx1250, now benchmarked beside direct and implicit-GEMM.

Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted

Follow-up review: first pass by a local qwen3.6 model, verified by the orchestrator (Claude Opus 5.5). Checks: delta since our review, merge with today's develop, build + ctest, byte identity ×4 flavors, golden gate and the saved alias check, differential golden, PR tests (source tree and installed layout), a host-side run of the compare, sweep and shard paths with the GPU stubbed, compile, CI at head. Flags whether a closer look is needed; not an approval or rejection.

cgmillette

lgtm

assistant-librarian · 2026-11-03T11:08:22Z

TheRock Submodule Bump Activity

Newest first

2026-11-03 — Included in TheRock via ROCm/TheRock#9698.

jakpiase requested review from a team as code owners October 30, 2026 21:19

github-actions Bot added documentation project: hip-kernel-provider rocke: platform Touches rocKE platform labels Oct 30, 2026

jakpiase requested a balanced review from Copilot October 30, 2026 21:26

Copilot started reviewing on behalf of jakpiase October 30, 2026 21:27

View session

assistant-librarian Bot added the organization: ROCm label Oct 30, 2026

Copilot AI reviewed Oct 30, 2026

View reviewed changes

bartekxk reviewed Oct 30, 2026

View reviewed changes

Comment thread dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/benchmark_winograd_aot.py Resolved

jakpiase and others added 2 commits November 1, 2026 16:49

Merge remote-tracking branch 'origin/develop' into users/jakpiase/roc…

ff9284d

…ke/gfx1250-winograd-aot

bartekxk approved these changes Nov 2, 2026

View reviewed changes

jakpiase requested a review from cgmillette November 2, 2026 15:46

cgmillette approved these changes Nov 2, 2026

View reviewed changes

jakpiase merged commit c91cf58 into develop Nov 2, 2026
166 of 169 checks passed

jakpiase deleted the users/jakpiase/rocke/gfx1250-winograd-aot branch November 2, 2026 20:06

jakpiase mentioned this pull request Nov 2, 2026

feat(rocke): gfx1250 WMMA direct fwd: 1-D, rectangular, stride-2 views, small-K, depthwise, small-group and dilated #14835

Merged

1 task done

Conversation

jakpiase commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Checks Passed — Ready for Review

Uh oh!

therock-pr-bot Bot commented Oct 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cgmillette commented Oct 31, 2026

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 First-round: wide-C fused K2 spills, untested

Uh oh!

jakpiase commented Oct 31, 2026

Uh oh!

cgmillette commented Oct 31, 2026

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🔴 Follow-up: llvm24 alias breaks goldens

Uh oh!

jakpiase commented Nov 1, 2026

Uh oh!

cgmillette commented Nov 1, 2026

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 Follow-up: alias fixed, CI running

Uh oh!

jakpiase commented Nov 1, 2026

Uh oh!

cgmillette commented Nov 1, 2026

PR #14791 — shape-generic (AOT) Winograd forward transforms for gfx1250 (jakpiase, AICK-2884) — 🟡 Follow-up: compare integration verified, CI running

Uh oh!

cgmillette left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

assistant-librarian Bot commented Nov 3, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jakpiase commented Oct 30, 2026 •

edited

Loading

therock-pr-bot Bot commented Oct 30, 2026 •

edited

Loading