Repository navigation
test(rocke): split pinned GPU references by operation, architecture, and variant - #14164
Conversation
Verification builds each replay request with the current harness, but published runners are frozen, so a request change could break them on their GPU only. A host test now replays every enrolled case of each published bundle through run_worker, as a script and a reused worker, with a stand-in runtime, sending today's request and one with an added variant key. It skips, naming the missing data, only without a bundle. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each baseline lock now records cohort_sha256, a digest of every qualified case contract, input digest and tolerance. Configuration checks it against the pinned manifest, qualify writes it, and an installed host test compares it with each adapter's CASES, so an unrequalified cohort edit, such as to the shared WMMA cases, fails without bundle data. The six locks gain their manifests' digests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The conv and SDPA architectures packages duplicated enrollment and lock lookup. Both now delegate to reference_common/registry.py, a standard library module that qualification freezes into new bundles, with the same names, errors and import order. Frozen-runner tests require the registry to load from the runner, and a host test checks enrollment. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each reference adapter and baseline lock moves to a variant directory named for its kernel family, and the processor package keeps only NAME. The registry finds variants without importing them, the worker imports the requested variant's adapter only in source mode, and pinned tests run per (architecture, variant). Archives and installed bundles stay per architecture, so configuration requires exactly one locked variant. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Each published variant now has its own DVC pointer and archive under reference_bundles/<op>/<arch>/<variant> and installs below its architecture's directory, so TheRock still splits payloads by processor. artifact.py --variant rejects a manifest declaring another variant. An architecture may publish several variants; its GPU CTest runs them all with a scaled timeout. One .gitignore now ignores every variant archive. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every configuration, with references on or off, now fails on a pre-variant lock or pointer, an unpaired lock or pointer, a lock outside its architecture's list or without an adapter, an invalid variant name, or a nested ignore file, so a stale merge cannot drop a qualification. Host tests check variant names, modules and NAMEs and that processor packages import no variant; a lint holds the module to CMake 3.16. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
qualify, verify, cases and diff take --variant, which defaults to an architecture's only variant and lists the choices when ambiguous; cases groups variants and names shared cohorts, and diff checks every published variant. --rocke-reference-variant selects one in pytest, where an unpublished variant runs only a candidate and an empty selection fails. Rerun and unpack hints always name the variant. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Qualification now writes a top-level "variant" for every operation, and verify and diff refuse a manifest that declares another variant than the one selected. Manifests without it, all published ones, stay valid and are identified by their lock. Staging reports a declared mismatch as the lock's, without a fetch hint that cannot help. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Add a host-only status command that prints one line per enrolled variant: its publication and baseline, the variants sharing its cohort, and whether it matches, differs, is missing or is unusable. A lock that pins another cohort's digest differs even without bundle data. It exits 1 on a difference and 2 on unusable data. artifact.py published lists committed variant locks so tools need not build paths. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The guides still described one reference per operation and architecture. Define variants, published variants, and the cohort digest; add recipes for a new variant or architecture and for un-sharing the WMMA cohort, naming the host tests that fail until an edited cohort is requalified; and update paths, --variant options, the status and published commands, configure guards, and triage rows. Record when a kernel family is a variant rather than an operation, what concurrent pull requests still share, and how checkouts, branches, and external tools move from the old layout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
a81da6c to
222c73b
Compare
|
🎉 All checks passed! This PR is ready for review. |
There was a problem hiding this comment.
🔵 Needs a closer look
The cross-architecture bundle migration and frozen-runner compatibility require maintainer validation, alongside resolution of the retained findings.
2 open issues
What changed in this PR
This PR splits rocKE’s pinned convolution and SDPA references by operation, architecture, and kernel variant, allowing kernel families to maintain independent baselines without rewriting shared reference files.
Changes:
- Moves adapters, locks, and DVC pointers into variant-specific paths while preserving published archive identities.
- Adds variant-aware discovery, selection, validation, and installation, retaining one GPU CTest per operation/architecture.
- Expands host checks and documents qualification, publication, and migration workflows.
Paths below are relative to dnn-providers/hip-kernel-provider/rocke/.
| File | Description |
|---|---|
TESTING.md |
Documents variant-based coverage and installation. |
platform/tests/test_gpu_references.py |
Tests variant staging, publication, and configuration guards. |
platform/tests/README.md |
Updates installed reference-suite descriptions. |
platform/CMakeLists.txt |
Scales GPU CTest timeouts by variant count. |
platform/cmake/PublishedGpuReferences.cmake |
Clarifies publication semantics in comments; lists remain unchanged. |
platform/cmake/InstallGpuReferences.cmake |
Validates reference wiring and installs variant bundles. |
library/tests/test_sdpa_torch_reference.py |
Checks variant identity and qualification cohort digests. |
library/tests/test_sdpa_reference_contract.py |
Adapts SDPA contract checks to variant adapters. |
library/tests/test_conv_reference_contract.py |
Adapts convolution contract and qualification checks. |
library/tests/sdpa_reference/worker.py |
Separates source adapter selection from processor-only replay. |
library/tests/sdpa_reference/torch_reference.py |
Uses the variant adapter lookup for oracle audits. |
library/tests/sdpa_reference/docs/sdpa-test-reference.md |
Documents SDPA variants and updated bundle paths. |
library/tests/sdpa_reference/architectures/gfx950/dense/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/sdpa_reference/architectures/gfx950/dense/__init__.py |
Houses the relocated gfx950 dense adapter. |
library/tests/sdpa_reference/architectures/gfx950/__init__.py |
Retains processor identity without importing variants. |
library/tests/sdpa_reference/architectures/gfx942/dense/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py |
Houses the relocated gfx942 dense adapter. |
library/tests/sdpa_reference/architectures/gfx942/__init__.py |
Retains processor identity without importing variants. |
library/tests/sdpa_reference/architectures/gfx1201/wmma/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/sdpa_reference/architectures/gfx1201/wmma/__init__.py |
Houses the relocated gfx1201 WMMA adapter. |
library/tests/sdpa_reference/architectures/gfx1201/__init__.py |
Retains processor identity without importing variants. |
library/tests/sdpa_reference/architectures/gfx1200/wmma/__init__.py |
Houses the unpublished gfx1200 WMMA adapter. |
library/tests/sdpa_reference/architectures/gfx1200/__init__.py |
Retains processor identity without importing variants. |
library/tests/sdpa_reference/architectures/gfx1151/wmma/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/sdpa_reference/architectures/gfx1151/wmma/__init__.py |
Houses the relocated gfx1151 WMMA adapter. |
library/tests/sdpa_reference/architectures/gfx1151/__init__.py |
Retains processor identity without importing variants. |
library/tests/sdpa_reference/architectures/__init__.py |
Delegates SDPA lookups to the shared registry. |
library/tests/reference_common/worker.py |
Documents source and replay target roles. |
library/tests/reference_common/registry.py |
Adds shared processor and variant discovery. |
library/tests/reference_common/pytest_support.py |
Validates and filters explicit variant selections. |
library/tests/reference_common/pinned_tests.py |
Runs comparisons and failure controls per variant. |
library/tests/reference_common/paths.py |
Resolves variant-specific source and installed bundles. |
library/tests/reference_common/docs/README.md |
Adds variant terminology and migration guidance. |
library/tests/reference_common/docs/quickstart.md |
Adds variant authoring and execution recipes. |
library/tests/reference_common/docs/gpu-reference-workflow.md |
Updates qualification, publication, and troubleshooting procedures. |
library/tests/reference_common/docs/gpu-ci-pinned-rocke-test-reference-plan.md |
Updates reference architecture and storage documentation. |
library/tests/reference_common/artifact.py |
Adds cohort validation, variant checks, and publication listing. |
library/tests/reference_bundles/sdpa/gfx950/dense.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/sdpa/gfx942/dense.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/sdpa/gfx1201/wmma.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/sdpa/gfx1151/wmma.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/sdpa/.gitignore |
Removes operation-local archive ignore rules. |
library/tests/reference_bundles/conv/gfx950/implicit_gemm_fwd.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc |
Updates the archive filename while preserving its hash and size. |
library/tests/reference_bundles/conv/.gitignore |
Removes operation-local archive ignore rules. |
library/tests/reference_bundles/.gitignore |
Centralizes variant archive and extracted-bundle ignore rules. |
library/tests/conv_reference/worker.py |
Separates source adapter selection from processor-only replay. |
library/tests/conv_reference/docs/conv-test-reference.md |
Documents convolution variant ownership and paths. |
library/tests/conv_reference/check_torch_reference.py |
Uses variant adapter lookup in oracle checks. |
library/tests/conv_reference/architectures/gfx950/implicit_gemm_fwd/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/conv_reference/architectures/gfx950/implicit_gemm_fwd/__init__.py |
Houses the relocated gfx950 forward adapter. |
library/tests/conv_reference/architectures/gfx950/__init__.py |
Retains processor identity without importing variants. |
library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/baseline_lock.json |
Adds the qualified cohort digest. |
library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py |
Houses the relocated gfx942 forward adapter. |
library/tests/conv_reference/architectures/gfx942/__init__.py |
Retains processor identity without importing variants. |
library/tests/conv_reference/architectures/__init__.py |
Delegates convolution lookups to the shared registry. |
library/tests/conftest.py |
Registers the pytest variant-selection option. |
Review effort: Balanced
Add instructions for packing against reviewed lock. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14164 — split pinned GPU references by operation, architecture and variant — 🟢 First-round: no closer look needed — the six published bundles move byte for byte without requalification, every host check passes from a source checkout, and the moved gfx942 bundles pass on the GPU in CI
No net-new findings. Utility: 🟢 Positive — independent kernel work on one architecture now lands in disjoint files with its own baseline, so pinning the next kernel family no longer requalifies every case on that architecture. Configuration also refuses half-wired or pre-variant references instead of silently dropping their coverage. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted First-round review: a local qwen3.6 model made the independent first pass. The orchestrator (Claude Opus 5.5) verified it. The checks were the reference host suites, the torch audit and the platform CMake tests on the merge and on develop, the DVC pointer diff, the |
…utput The guide listed paged KV as a new operation, although paged K/V on cases that SDPA's Case describes has the same oracle, inputs, and output. A variant is now defined by the operation's Case, input generator, independent reference, single output, and manifest id, including a kernel that also reads read-only operands derived from the case. The guide states that an operation has one case model, the tensors a case reads and writes and how their shapes follow from its fields, and gives the rule: a per-case knob that keeps that model, such as a sliding window, is a new Case field, and other math, another output, or another case model, such as backward, an LSE output, per-sequence lengths, or decode, is a new operation. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
One GPU CTest per operation and architecture keeps the CTest names and the provider category patterns, at no cost while every architecture has one variant. The workflow guide now records what it gives up with two or more variants (one verdict, a hang that can use the whole scaled timeout, whole-architecture exclusions), and says that an architecture's entry runs at most two variants: before a third, or once one variant needs its own exclusion or category, each variant gets its own CTest with a timeout longer than the worker's request timeout, and the category patterns are widened. The quickstart's add-a-variant recipe and the concurrency table say the same. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since d57e6c5a382c
ROCm/rocm-libraries#14164 · eee5958744b0 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 1, MEDIUM 0, LOW 0, NEEDS-INFO 13
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (d57e6c5a382c)
Resolved 0 · still open 1 · new 0
- ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
Findings
- HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892· Put the knob on the KernelSpec and delete the environment read.
Needs info
- §B.4 At CLI, pytest, and CMake trust boundaries for the new variant parameter and lock files, invalid or ambiguous inputs are explicitly rejected rather than normalized into another valid variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:77). — no diff line supports this verdict
- §C.1 Re-review resolves the previous NEEDS-INFO: repeated per-operation architecture/variant lookup is now a named shared helper in
dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and both operation packages delegate to it indnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict - §C.7 Re-review resolves the previous NEEDS-INFO: the bundle move is shown as a no-op archive identity move by preserving the DVC object hash while changing only the archive path, e.g.
dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:2anddnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:5. — no diff line supports this verdict - §E.13 Several diffs (
cli.py,test_reference_common.py,test_gpu_references.py, anddocs/*.md) are truncated or omitted in the prompt, so full removal of old per-architecture references across all files cannot be verified from the provided diff alone. - §E.15 Previously NEEDS-INFO; verified in the current diff that new
variantparameters are read and used invalidate_bundle(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read atdnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143),pack(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:203),unpack(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:293), andAdapterRegistry.resolve_variant(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65, read at… - §F.6 Local
pre-commitexecution status before push is not visible from the diff alone. - §F.7 Bot and Copilot review threads and their resolution status are not included in the prompt.
- §G.3 Missing current-head evidence that the required local non-GPU gate sweep was run, or a baseline-diff showing the affected gates are unchanged versus develop.
Section notes
- §D: This PR reorganizes pinned GPU reference test bundles and harnesses by (operation, architecture, variant); it does not touch dispatcher registration, routing, selectors, or KernelSpec definitions.
- §D: This PR only restructures pinned GPU test reference harnesses by (operation, architecture, variant) and does not touch the production dispatcher, candidates, or kernel selectors.
- §G: Items 18–34 of 34
- §H: No device-side masks, bounds, indirection tables, or kernel implementations are added or modified in this PR; it only reorganizes test reference bundle layouts, locks, DVC pointers, and Python/CMake test harness infrastructure.
- §J: Markdown present. Shapes: | Doc | Operands / inner dimensions | Result vs declared | |---|---|---| |
library/tests/conv_reference/docs/conv-test-reference.md| Documents tensor layouts (NHWC,KYXCwithC/groups,NHWK) but adds no equation using them. | No equation result shape is declared. | |TESTING.md,platform/tests/README.md, omitted docs | No visible operand shape annotations in the provided diff. | N/A. | Symbols: | Symbol | Definition / domain | |---|---| |architecture/arch| Processor-level GPU reference selection and package level. | |variant| Kernel-family reference adapter/bundle level under an architecture. | |cohort| Enrolled case set associated with a lock/bundle identity. | Novelty: | Claim | Check status | |---|---| | Conv now publishes theimplicit_gemm_fwdvariant and lists it viastatus. | Visible in the provided diff, but complete develop comparison is blocked by omitted Markdown hunks. | | Pinned gradients would be a separate… - §K: This PR reorganizes pinned GPU test reference bundles by (operation, architecture, variant); it does not touch dispatch tuning, KnobSpace subclasses, knob axes, sweep entry points, or benchmark tuning replay.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR restructures pinned GPU reference harnesses and bundles, but does not add or modify kernel-body emitters. |
| 2 | N/A | This PR does not modify core IR or lowering-op handler paths such as core/ir or core/lower_*. |
| 3 | N/A | This PR changes reference-test adapters and publication plumbing, not production kernel-level selectors, spec builders, lowering, or codegen knobs. |
| 4 | N/A | No Python or C++ IR emitter code is changed, so emit-order parity does not apply. |
| 5 | N/A | The diff does not modify representative-IR golden files or intentional kernel IR output. |
| 6 | N/A | There is no engine or emitted-IR change in this PR requiring golden-vs-cross-engine parity proof. |
| 7 | N/A | The new logic is in pinned-reference test selection, packaging, and installation rather than a kernel or lowering hot path that emits IR. |
| 8 | N/A | The PR does not touch C++ engine, builder, selector, or lowering implementation code. |
| 9 | N/A | The PR contains no shared op-handler fix or lowerer-only architecture divergence. |
| 10 | N/A | The PR does not introduce a new kernel/compiler feature flag that changes emitted IR. |
| 11 | N/A | The PR does not add a new IR opcode. |
| 12 | N/A | The PR does not modify ROCKE_BACKEND behavior or core/backend.py::CPP_UNPORTED_ARCHES. |
| 13 | N/A | The PR does not change emitted .ll generation or ROCKE_DEBUG_LOC handling. |
| 14 | N/A | The PR does not modify classic helpers under platform/cpp/helpers or their C++ helper headers; helpers/tiling is not involved. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only touches test reference infrastructure and documentation, with no new kernel or dispatch rejection gates. |
| 2 | N/A | No preflight gates or kernel spec constructors are added or modified in this PR. |
| 3 | N/A | No model-zoo dispatch routing or kernel fallback logic is changed in this test-only PR. |
| 4 | NEEDS-INFO | At CLI, pytest, and CMake trust boundaries for the new variant parameter and lock files, invalid or ambiguous inputs are explicitly rejected rather than normalized into another valid variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:77). — no diff line supports this verdict |
| 5 | N/A | No kernel dispatch gates or supported configuration scopes are modified in this PR. |
| 6 | N/A | No is_valid_spec or supports_* functions are touched in this PR. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Re-review resolves the previous NEEDS-INFO: repeated per-operation architecture/variant lookup is now a named shared helper in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and both operation packages delegate to it in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict |
| 2 | N/A | This PR refactors pinned GPU reference test layout and staging, not kernel build_<family>(spec) builders or emitted IR. |
| 3 | PASS | Variant names are operation/algorithm family names such as implicit_gemm_fwd in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4, dense in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:4, and CMake rejects invalid or gfx-prefixed variant names in… |
| 4 | PASS | Arch-independent registry and lookup logic lives in the common helper dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, while processor packages now only name the enrolled processor, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/__init__.py:9, and algorithm-specific adapters live under variant subpackages such as… |
| 5 | PASS | The diff fan-out matches the inherent reference-key widening across tests, bundle paths, CMake staging, and docs: default_bundle_path gains a variant component in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, CMake installs variant payloads in dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:205, and TESTING documents the new installed path in… |
| 6 | PASS | Re-review resolves the previous NEEDS-INFO: the easy change is factored through the shared AdapterRegistry abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22 before operation packages bind get_adapter, variants, and baseline_lock to that abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:13. |
| 7 | NEEDS-INFO | Re-review resolves the previous NEEDS-INFO: the bundle move is shown as a no-op archive identity move by preserving the DVC object hash while changing only the archive path, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:2 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:5. — no diff line supports this verdict |
| 8 | PASS | New functionality extends the existing shared reference tooling rather than reimplementing separate operation tools: artifact.py adds the published command in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:316, and CLI cohort/variant handling is added to ReferenceOperation in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:170. |
| 9 | PASS | The variant-source/replay split is treated as a class across sibling workers: convolution dispatches source mode through get_adapter in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:45, and SDPA applies the same source/replay guard in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:64; CMake also rejects stale pre-variant locks and pointers repo-wide in… |
| 10 | PASS | The widened shared key (operation, architecture, variant) is propagated through option parsing in dnn-providers/hip-kernel-provider/rocke/library/tests/conftest.py:55, path construction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, pytest parametrization in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:178, CMake staging in… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892 reads the environment |
| 2 | N/A | This PR does not add or modify any dispatcher KernelCandidate or Capability declarations. |
| 3 | N/A | This PR does not add, modify, or call any dispatcher candidate _supports or admits predicates. |
| 4 | N/A | This PR does not touch dispatcher selectors or spec builders on the selection path. |
| 5 | N/A | This PR does not modify any KernelSpec identity, kernel_name, or cache-key logic. |
| 6 | N/A | This PR does not modify cohort routing, spec builders, launch metadata, or cache keys in the dispatcher. |
| 7 | N/A | This PR does not add or modify dispatcher heuristics, hand-pinned overrides, or resolvers. |
| 8 | N/A | This PR does not define or modify any dispatcher routing cohorts or spec override predicates. |
| 9 | N/A | This PR does not register any new kernel or architecture in the dispatcher registries. |
| 10 | N/A | This PR does not introduce any new KernelSpec fields or features. |
| 11 | N/A | This PR does not register, promote, or change precedence or opt-in status for any dispatcher candidates. |
| 12 | N/A | This PR only restructures pinned GPU test reference harnesses and does not add, rename, or reprioritize any dispatcher candidates. |
| 13 | N/A | This PR does not modify CandidateRegistry.select or add any dispatcher wrappers or entry points. |
| 14 | N/A | This PR does not touch kernel compilation caches, routing logs, tuning records, or kernarg layouts. |
| 15 | N/A | This PR does not add or modify any dispatcher candidates or arch modules in the production dispatch registry. |
| 16 | N/A | This PR does not introduce a new operator family in the dispatcher. |
| 17 | N/A | This PR does not add or modify dispatcher candidates or routing rules. |
| 18 | N/A | This PR does not modify CandidateRegistry settings, binding requirements, or exempt sets. |
| 19 | N/A | This PR makes no claims about unified attention auto routing or run_unified_attention_torch dispatch. |
| 20 | N/A | This PR makes no changes to kernel-level selectors, spec builders, or the Python registry/tuning layer. |
| 21 | N/A | This PR does not fix a kernel selection or emitter issue; it reorganizes test reference bundles by variant. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | library/tests imports rocke (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:57), while platform/cmake/InstallGpuReferences.cmake invokes artifact.py via an isolated subprocess (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:208) without importing library into platform. |
| 2 | PASS | Device queries in the reference test infrastructure use from rocke.runtime.hip_module import get_device_target_id (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:59) rather than torch. |
| 3 | PASS | Torch remains optional: dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/check_torch_reference.py:13 gates with pytest.importorskip("torch") and pinned reference tests verify assert not report["torch_imported"] (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:210). |
| 4 | N/A | This PR only modifies files under library/tests/, platform/, and TESTING.md, not production library/{kernels,dispatch,builders,benchmarks} layering or KNOWN_VIOLATIONS/KNOWN_BAD lists. |
| 5 | PASS | Platform CMake rules use relative directory variables (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:29) and Python helpers use tempfile and pathlib (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:18). |
| 6 | N/A | This PR does not touch emitter trees (core/, helpers/, instances/, library/kernels/). |
| 7 | PASS | Testing and workflow documentation are updated alongside the reference layout changes (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/docs/conv-test-reference.md:13). |
| 8 | PASS | No Jira/tracking IDs, reviewer handles, or PR numbers appear in code or comments in the diff (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:1). |
| 9 | PASS | All modified files are scoped to splitting pinned GPU reference bundles by operation, architecture, and variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | The PR explicitly accounts for concurrent branches and merge collisions by adding CMake guards against pre-variant locks and DVC pointers restored at legacy paths (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107). |
| 11 | PASS | Only deliverable test harness code, locks, DVC pointers, CMake rules, and required reference documentation are committed (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22). |
| 12 | PASS | All enrolled architectures (gfx942, gfx950, gfx1151, gfx1200, gfx1201) are migrated to the variant layout (dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:471, dnn-providers/hip-kernel-provider/rocke/platform/cmake/PublishedGpuReferences.cmake:11). |
| 13 | NEEDS-INFO | Several diffs (cli.py, test_reference_common.py, test_gpu_references.py, and docs/*.md) are truncated or omitted in the prompt, so full removal of old per-architecture references across all files cannot be verified from the provided diff alone. |
| 14 | PASS | Moved adapter files preserve upstream comments, historical API compatibility checks, and docstrings (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:43). |
| 15 | NEEDS-INFO | Previously NEEDS-INFO; verified in the current diff that new variant parameters are read and used in validate_bundle (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143), pack (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:203), unpack (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:293), and AdapterRegistry.resolve_variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65, read at… |
| 16 | N/A | This item is a meta-instruction for review grading rather than a property of the pull request diff. |
| 17 | PASS | No performance numbers or superiority claims appear in the PR description, docs, or code (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/brockhargreaves-amd/pinned-reference-variants matches users/<user>/… (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
| 2 | PASS | title test(rocke): split pinned GPU references by operation, architecture, and variant matches type(scope): summary and is 80 characters |
| 3 | PASS | description is at least 30 characters and contains an issue reference |
| 4 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py is in the diff |
| 5 | PASS | no key, certificate, or secret filename in the diff |
| 6 | NEEDS-INFO | Local pre-commit execution status before push is not visible from the diff alone. |
| 7 | NEEDS-INFO | Bot and Copilot review threads and their resolution status are not included in the prompt. |
| 8 | PASS | Base branch is develop (though body notes Stacked on #13651), and the branch, title, body, tests, and files satisfy policy.yml checks (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312). |
| 9 | PASS | The PR keeps the full 10-commit migration and test suite together without artificial splitting for a size cap (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | Tests added in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:341 exercise real variant discovery, cohort digesting, and CMake staging behavior rather than throwaway checks. |
| 11 | PASS | The fetched PR metadata shows the populated Title and full Motivation/Technical Details/Test Plan Body are present (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refactors pinned GPU reference bundle layout and harness plumbing, not byte-identity parity as a correctness proof. |
| 2 | N/A | This PR does not add or modify a kernel implementation, so the kernel-work on-silicon done criterion does not apply. |
| 3 | NEEDS-INFO | Missing current-head evidence that the required local non-GPU gate sweep was run, or a baseline-diff showing the affected gates are unchanged versus develop. |
| 4 | PASS | New variant behavior is covered by falsifiable tests, e.g. selected declared variants are accepted/rejected in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:398 and missing/unpublished variant paths are exercised in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:807. |
| 5 | PASS | Tests exercise shipped surfaces rather than only unreachable internals, including the published artifact CLI via subprocess in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:309 and pytest session/selection behavior in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:807. |
| 6 | NEEDS-INFO | The diff is truncated and omits documentation hunks under reference_common docs, sdpa docs, platform CMakeLists, and platform/tests/test_gpu_references.py, so the load-bearing prose and its checks cannot be fully verified from the provided diff. |
| 7 | N/A | This PR does not introduce a new numerical oracle design doc or new fp32 parity-reference policy. |
| 8 | N/A | No dtype fallback behavior is added or modified in the shown harness changes. |
| 9 | N/A | This PR makes no LDS or occupancy resource claims. |
| 10 | N/A | This PR makes no performance-result or provenance claims that require device/script/ratio validation. |
| 11 | N/A | This PR does not add or optimize a kernel or dispatch selector whose reachability must be proven. |
| 12 | N/A | The changes are pinned-reference test infrastructure and do not introduce a two-engine emitted-IR or lowering parity gap. |
| 13 | N/A | No LLVM intrinsic or TableGen symbol is added, changed, or recommended. |
| 14 | N/A | No bottleneck classification or static-versus-dynamic performance analysis is introduced. |
| 15 | N/A | No wrong-branch risk is evident from the provided checklist scope for this infrastructure refactor. |
| 16 | NEEDS-INFO | Resolves the previous NEEDS-INFO: the shared registry defines and validates variant adapter lookup in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:81, and both operation architecture packages delegate their exported symbols to it, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/init.py:12 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/init.py:12. — no diff line supports this verdict |
| 17 | PASS | Shipped scripts are exercised end-to-end through subprocess tests, including artifact.py published in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:309 and common CLI round-tripping in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:207. |
| 18 | PASS | New configuration and cohort-digest guards are wired into CMake configuration and bundle validation (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:40) and are proven to fire on constructed fault conditions while passing valid states (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:588… |
| 19 | PASS | Test fixtures construct multi-variant, unpublished, half-wired, and mismatched-cohort shapes needed to express the failures (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1273, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:338). |
| 20 | N/A | This PR does not add or modify a detector banning performance or numerical claims. |
| 21 | N/A | This PR refactors reference bundle layout by variant and does not introduce performance or statistical measurement code. |
| 22 | PASS | The harness explicitly fails rather than skipping when a selected operation, variant, or published architecture has no bundle or no published variants (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:96, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1468), and the PR body confirms 0 skipped across all 6 hardware suites. |
| 23 | N/A | This PR makes no performance or routing speedup claims. |
| 24 | N/A | This PR does not re-bless IR goldens or change kernel codegen; the existing reference bundles are moved byte-for-byte with identical manifest_sha256… |
| 25 | N/A | This PR does not contain a codegen micro-optimization. |
| 26 | N/A | This PR does not benchmark kernels or report performance numbers. |
| 27 | N/A | This PR does not perform A/B performance comparisons or ship kernel optimization knobs. |
| 28 | N/A | This PR does not report performance deltas or counterfactual comparisons. |
| 29 | N/A | This PR does not swap math primitives in any kernel. |
| 30 | N/A | This PR does not make register pressure or zero-spill claims. |
| 31 | N/A | This PR does not claim to fix a kernel performance defect or bank conflict against a baseline. |
| 32 | N/A | This PR does not report measured performance or hardware resource figures. |
| 33 | N/A | This PR does not change any kernel dispatch resolver, heuristic, or spec builder decision function. |
| 34 | N/A | This PR does not add or modify an opt-in kernel output flag. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only reorganizes test reference bundles and CMake/Python test harness infrastructure without touching kernel specs, SRDs, or launch validation. |
| 2 | N/A | This PR does not add or modify paged/indirection kernels or device table lookups. |
| 3 | N/A | This PR does not add or modify any device kernel code or compile-time tile/page divisors. |
| 4 | N/A | This PR does not touch device-side masks, bounds, or runtime sequence-length inputs. |
| 5 | N/A | This PR does not introduce or modify kernel launch inputs feeding device-side bounds or masks. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refactors test reference bundle layout by variant and does not perform kernel routing or candidate space measurement. |
| 2 | PASS | Invalid or half-wired states are loudly rejected with actionable error messages and recovery commands rather than silently ignored or cryptically failing (e.g., dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:39-135 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143-173). |
| 3 | N/A | This PR splits existing pinned reference cohorts by variant without altering test case generation or hypothesis property tests. |
| 4 | N/A | This PR does not generate performance tables or benchmark reports comparing shipped vs achievable kernel selections. |
| 5 | PASS | Pinned reference bundles are explicitly tracked as frozen snapshots via baseline_lock.json (recording baseline_revision, cohort_sha256, and manifest_sha256) with an explicit qualification/regen CLI path and status/diff drift checks (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349-495, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:1261-1317). |
| 6 | N/A | This PR does not add or modify any DSL ops, hardware instructions, or lowering primitives. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The modified Markdown visible in the diff contains no mathematical equations or matrix products to type-check. |
| 2 | N/A | The PR does not introduce formula notation tables or multi-domain mathematical quantities such as decay versus log-decay. |
| 3 | N/A | The visible Markdown documents GPU reference packaging and workflows, not a kernel algorithm derivation owned by another doc. |
| 4 | NEEDS-INFO | Not resolved from the previous NEEDS-INFO: the diff still omits substantial Markdown hunks for library/tests/reference_common/docs/README.md, gpu-ci-pinned-rocke-test-reference-plan.md, gpu-reference-workflow.md, quickstart.md, sdpa-test-reference.md, and platform/tests/test_gpu_references.py-adjacent docs, so novelty claims there cannot be checked against develop. |
| 5 | PASS | TESTING.md:921-923 states the installed CTest timeout as a per-variant aggregate (five minutes per installed variant) rather than a bare per-test constant. |
| 6 | N/A | This PR reorganizes pinned test reference bundles and does not add model configuration tables, tuned dispatch band edges, or similar constants requiring deployment provenance. |
| 7 | N/A | No new Markdown design or algorithm document file is added in the provided diff; existing test documentation is updated. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any tuned dispatch candidates or KnobSpace subclasses. |
| 2 | N/A | This PR does not add or modify any kernel spec fields or KnobAxis declarations. |
| 3 | N/A | This PR does not change tuning_id, config_key, knob canonicalization rules, or tuned spec defaults. |
| 4 | N/A | This PR does not touch KnobSpace.defaults() or KnobSpace.recorded(). |
| 5 | N/A | This PR does not modify knob dropping, refusal, inertness, or base_value() rules. |
| 6 | N/A | This PR does not modify KnobSpace.validate or policy-free tuning spec construction. |
| 7 | N/A | This PR does not touch KnobSpace.base(req) or request normalization for tuning. |
| 8 | N/A | This PR does not change normalize_knobs or knob value coercion. |
| 9 | N/A | This PR modifies GPU test reference bundle replay, not dispatch tuning_id/spec_hash benchmark replay. |
| 10 | N/A | This PR does not touch KnobSpace.find or tuning_id resolution in dispatch. |
| 11 | N/A | This PR does not touch sweep entry points, pin_to_spec, or sweep harnesses. |
| 12 | N/A | This PR does not change production stacks, accept_default, or base specs for auto tuning selection. |
| 13 | N/A | This PR does not touch benchmark or sweep harnesses or make claims about auto selection. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR reorganizes pinned GPU test reference bundles by variant and does not add or modify any gfx hardware target or ISA backend. |
| 2 | N/A | This PR does not add or modify any MMA op_id or layout map. |
| 3 | N/A | This PR does not touch MMA selection, MfmaAtom catalogs, or kernel lowering. |
| 4 | N/A | This PR only relocates existing test adapter code and does not alter ArchTarget.wave_size or lane math. |
| 5 | N/A | This PR does not add or modify arch-specific kernel recipes or tile loaders. |
| 6 | N/A | This PR does not modify pipeline policies, ArchTarget, or rocke/ imports. |
| 7 | N/A | This PR does not add or modify logical dtypes, bit packing, instruction fragments, or FP8 dialects. |
| 8 | N/A | This PR does not touch scaled-MMA contracts or WMMA/MFMA scaled emission. |
| 9 | N/A | This PR does not introduce a new architecture target. |
| 10 | N/A | This PR does not modify intrinsic arch-domain artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not touch DSL kernel bodies, SSA Values, or runtime predicate/control-flow emission. |
| 2 | N/A | The diff does not touch legalize_for_target or tile/MMA lowering. |
| 3 | N/A | The diff does not touch ck.dsl.ir serialization, print_ir, wire versions, attributes, or ABI parameter ordering. |
| 4 | N/A | The diff does not touch portable IR schemas, importers, magic division helpers, recipe VM, or recorder coverage. |
| 5 | N/A | The diff does not touch AsyncTileLoader, LDS layouts, or waitcnt placement. |
| 6 | N/A | The diff does not emit GPU barriers, divergent control flow, memory loads, or cross-lane shuffles. |
| 7 | N/A | The diff does not touch ISA backend buffer resource words or TemporalHint lowering. |
| 8 | N/A | The diff does not add LLVM intrinsic declarations or modify compiler backend selection precedence. |
| 9 | N/A | The diff does not touch public C entry points, cpp/include/rocke/ headers, or C++ codegen launch behavior. |
| 10 | N/A | The diff does not touch debug-info emission or DWARF consumers. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add or modify helpers under platform/python/rocke/helpers/ or introduce a new helper promoted to helpers/. |
| 2 | N/A | The diff does not author or modify kernels or their use of shared helper mechanisms such as atoms, TensorView, loaders, epilogues, or transforms. |
| 3 | N/A | The diff does not touch SoftwarePipeline or any kernel pipeline code. |
| 4 | N/A | The diff does not add or modify fused kernels, epilogue fusion logic, fusion validation, or fusion legalizers. |
| 5 | N/A | The diff does not introduce or change any public tiling API under helpers/tiling/. |
| 6 | N/A | The diff does not add or modify front-door or toolbox tiling helpers or any LayoutStyle implementation. |
| 7 | N/A | The diff does not touch tiling layout overrides, accumulator descriptors, or MMA validation. |
| 8 | N/A | The diff does not touch tiling memory operations, LDS allocation, global load/store emission, clipping, padding, or wide-load selection. |
| 9 | N/A | The diff does not use or modify N-D tensor axis roles, batch reductions, flow-stage relabeling, or AB-swap routing in tiling. |
| 10 | N/A | The diff does not modify tiling emit, LDS staging, MMA issue logic, or tiling fail-fast raises. |
| 11 | N/A | The diff does not touch MMA traits, TileMma, lds_swizzle, scheduling group helpers, or related tiling policies. |
| 12 | N/A | The diff makes no LDS bank-conflict or coalescing claims and does not modify related simulator or coalescing analysis tools. |
| 13 | N/A | The diff does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | The diff does not use tiling verbs, modify synchronization or barrier placement, or add a tiling pipeline variant. |
| 15 | N/A | The diff does not touch tiling allocation or bounds-checking logic for transformed address ranges. |
| 16 | N/A | The diff does not use or modify optimize_layout, recommend, or evaluate_transform. |
| 17 | N/A | The diff does not touch the tiling recorder, round-trip analysis, witness flags, or visualization modules. |
| 18 | N/A | The diff does not touch interleaved layouts, tiling demo kernels, dispatch registration of tiling demos, or tiling numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not touch core/verify.py, cpp/core/verify.cpp, or IR address spaces/attributes. |
| 2 | N/A | This PR does not modify scf.for, region structure, or IR block emission. |
| 3 | N/A | This PR does not modify IRBuilder, LLVM intrinsic declaration tables, or metadata emission. |
| 4 | N/A | This PR does not modify IR types, SmemType, or IR parsers. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in Python or C++ passes. |
| 7 | N/A | This PR does not modify kernel attributes, COMGR options, or cluster dimension checks. |
| 8 | N/A | This PR does not touch CodegenPolicy, SchedulerStrategy, or kernel compilation options. |
| 9 | N/A | This PR does not modify waitcnt encoding or barrier lowering. |
| 10 | N/A | This PR does not touch LLVM lowerers or parameter attribute/type lowering. |
| 11 | N/A | This PR does not touch ArchTarget or target catalog resolution. |
| 12 | N/A | This PR does not touch vector/LDS width caps or codegen arch defaults. |
| 13 | N/A | This PR does not touch public C headers under cpp/include/rocke/ or ABI versions. |
| 14 | N/A | This PR does not touch C++ engine sources, headers, bindings, or C++ compile flags in platform/CMakeLists.txt. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or alternative lowerers. |
| 16 | N/A | This PR does not touch known_gaps.md or C++ library/builder capabilities. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No kernel builders, specs, or REQUIRED_FIELDS are added or modified in this PR. |
| 2 | N/A | No kernel argument signatures, ABI orders, or packing routines are modified in this PR. |
| 3 | N/A | Launch geometry logic is unchanged; reference adapters only relocate existing prepare/launch wrappers into variant subpackages. |
| 4 | N/A | No LDS capacity gates or emitters in Python or C++ are touched by this PR. |
| 5 | N/A | No WMMA/RDNA GEMM, GemmMultiD, or GemmMultiAbd instance specs or validators are modified in this PR. |
| 6 | N/A | No caller-zeroed workspaces (Stream-K, MoE, split-K) or workspace sizing helpers are touched in this PR. |
| 7 | N/A | No Stream-K, persistent GEMM, GroupedGemmLauncher, or SUPPORT_MATRIX.md limits are modified in this PR. |
| 8 | N/A | No quantization helpers, MX GEMM scales, or quantize_scalar_f32 paths are touched in this PR. |
| 9 | N/A | No numeric emulation or decode paths are added or changed in this PR. |
| 10 | N/A | This PR updates test reference bundle manifests (schema 2 under reference_common), not ck.dsl.example.manifest/v1 runtime manifests in helpers/manifest.py. |
| 11 | N/A | No runtime launch lifecycle, stream synchronization, WorkspacePool, or timing utilities are modified in this PR. |
| 12 | N/A | No COMGR or HIP library loading code or hipError launch fixes are included in this PR. |
| 13 | N/A | No production host-side launch math, tile-count helpers, or kernel-cache admission guards are refactored in this PR. |
| 14 | N/A | No examples/expected.json files, instances/, or helpers/ kernel modules are added or modified in this PR. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The diffs for dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt (+7 -2 lines) and dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py (+485 -80 lines) remain omitted to fit the model budget, so CTest registration and installed test changes cannot be verified. |
| 2 | N/A | This PR refactors the GPU reference test bundle layout and does not add a new kernel family or variant to the platform IR or builders. |
| 3 | PASS | Tests and documentation grade the reference harness accurately, enforcing failure controls for perturbed outputs and missing launches in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:168-276 and noting subset verification caveats in dnn-providers/hip-kernel-provider/rocke/TESTING.md:154-156. |
| 4 | N/A | This PR does not re-bless IR goldens or widen the valid spec space in a lowerer or builder. |
| 5 | PASS | Tests remain portable across CPU/GPU and OS environments, skipping cleanly when hardware or bundles are absent (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:76-160) and querying the active device processor dynamically (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pytest_support.py:80-115). |
| 6 | PASS | The harness refactor preserves per-configuration semantics, scaling timeouts per installed variant (dnn-providers/hip-kernel-provider/rocke/TESTING.md:924-925), keeping at most one slot's workers alive at a time (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:82-163), and preserving error classes (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:227-232). |
| 7 | N/A | This PR does not modify engine backend rejection contracts or ROCKE_BACKEND / ROCKE_CPP_STRICT fallback behavior. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only restructures pinned GPU test reference harnesses and bundles and changes no kernel body, algorithm, tiling strategy, pipeline variant, or performance lever. |
| 2 | N/A | No performance lever or architecture-specific optimization recipe is modified in this test-reference refactoring PR. |
| 3 | N/A | No optimization lever covered by the utilities skills is touched by this test-infrastructure PR. |
| 4 | N/A | The diff does not alter kernel codegen, lowering, or performance primitives, so static ISA/occupancy probe evidence does not apply. |
| 5 | N/A | No global memory accesses in GPU kernels are added or modified by this diff. |
| 6 | N/A | No LDS or MMA layouts are added or changed in this pull request. |
| 7 | N/A | The PR makes no kernel bottleneck claims and implements no performance fixes. |
| 8 | N/A | No waits, barriers, or scheduling hints in GPU kernels are changed. |
| 9 | N/A | This PR is not a performance regression fix and changes no shape-dependent performance lever. |
| 10 | N/A | No compiler codegen changes, assembly parity claims, or performance benchmarks are included in this diff. |
| 11 | N/A | This PR introduces no new GPU optimization lessons, probes, or analysis tools. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add any public symbol, spec, builder, compile/launch parameter, or C entry point to the rocke package or C engine. |
| 2 | NEEDS-INFO | Existing public API test coverage in library/tests/ and platform/tests/ is preserved without deleting, skipping, or narrowing tests; the cohort preservation guards in dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py:393 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:67 are updated to resolve variant adapters while keeping all existing test cases intact. — no diff line supports this verdict |
| 3 | N/A | No public API in rocke or the C engine is removed or renamed. |
| 4 | N/A | No __all__ list or public rocke module is modified in this diff. |
| 5 | PASS | New and updated tests in library/tests/ assert concrete outputs, digests, and fail-fast error raises, such as cohort digest and lock validation at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:540 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:588. |
| 6 | PASS | CPU-checkable contracts (manifest validation, cohort digests, CLI commands, and CMake reference wiring) are tested on CPU without Torch or a GPU at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61, while GPU launch and numeric comparisons are marked @pytest.mark.gpu at… |
| 7 | N/A | No API with a C++ twin or C ABI binding is added or modified. |
| 8 | PASS | No rocke public API or rocke.run_manifest code is modified, the PR test results name each GPU architecture (gfx942, gfx950, gfx1201, gfx1151), and the reference installation and contract suites are exercised across dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/TESTING.md:123. |
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14164 — split pinned GPU references by operation, architecture, and variant (brockhargreaves-amd, AICK-2299) — 🟡 Follow-up: fixes verified; minor items open
Net-new: none. Utility: 🟢 Positive — independent per-variant baselines without requalification; review gaps closed with tests. Legend: 🔴 blocking · 🟠 should-fix · 🟡 minor nit · 🟢 info/good · ⚪ retracted Follow-up review: a local qwen3.6 model made the independent first pass, and the orchestrator (Claude Opus 5.5) verified it. It read the 13 commits since our first round ( |
|
Doing another run on other architectures here: https://github.com/ROCm/rocm-libraries/actions/runs/41128621898 |
|
Follow-ups from @AnaghaRaoAMD's review. They are owned by the rocKE team (@ROCm/rocke-lib, 1. One GPU CTest per (operation, architecture, variant). From the CTest thread.
2. Add a
3. Split
4. Optional: a pinned reference for #14552's paged K/V kernels. From the operation-or-variant
GPU status of this PR at
|
|
Thanks @yraparti. The HIGH §D.1 is answered on its thread. As before, the 13 NEEDS-INFO rows concern hunks that the truncated diff left out. My 10-27 answers still hold at
|
|
Thanks @cgmillette. On the open items:
|
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since eee5958744b0
ROCm/rocm-libraries#14164 · 01e4921ab16f · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 2, MEDIUM 0, LOW 0, NEEDS-INFO 60
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (eee5958744b0)
Resolved 0 · still open 1 · new 1
- ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
- 🆕 §B.4 new FAIL (HIGH): In both reference workers, require
variantto be present for source-mode requests and passrequest["variant"]toget_adapter, while leaving replay mode independent of the variant adapter.
Findings
- HIGH §B ·
dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46· In both reference workers, requirevariantto be present for source-mode requests and passrequest["variant"]toget_adapter, while leaving replay mode independent of the variant adapter. - HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892· Put the knob on the KernelSpec and delete the environment read.
Needs info
- §A.1 The changes-since list names new or modified kernel-body emitter files under library/kernels/common/ and C++ instance files under platform/cpp/instances/common/, but their hunks are omitted, so I cannot verify whether every non-exempt Python family has a synchronized C++ twin or a documented known_gaps entry.
- §A.2 The changes-since list names core IR and lowering files in both platform/python/rocke/core/ and platform/cpp/core/, but the relevant hunks are omitted, so I cannot verify mirrored op-handler/lowering behavior.
- §A.3 The changes-since list names attention_unified.py and attention_unified_selectors.cpp plus related headers, but their hunks are omitted, so I cannot verify selector/spec/lowering/codegen parity.
- §A.4 The changed Python and C++ emitter hunks are omitted, so I cannot verify exact emit order, operand binding order, or canonical IR equality.
- §A.5 The changes-since list names representative and library golden JSON changes, but their contents and the corresponding IR-changing hunks are omitted, so I cannot tell whether each re-bless is intentional and appropriate.
- §A.6 The diff shown does not provide the cross-engine byte-identity gate result or identify whether golden changes were only Python-drift goldens versus check_byte_identity coverage.
- §A.7 The hot-path kernel and lowering hunks that would show whether new logic is guarded or unconditional are omitted.
- §A.8 The current diff includes C++ engine/instance/lowering file changes in the changes-since list, but no check_byte_identity result or complete C++ hunks are provided to review by hand.
Section notes
- §A: The re-review diff is truncated and the changes-since list names production rocKE kernel, core, lowering, backend, helper, parity, and golden files whose hunks are omitted; items that depend on those hunks are marked NEEDS-INFO.
- §B: The provided diff is truncated; rows marked NEEDS-INFO identify the missing hunks needed to grade safely.
- §D: This PR reorganizes pinned GPU reference test bundles and harnesses by (operation, architecture, variant) and does not touch dispatcher registration, routing, or KernelSpec definitions.
- §D: This PR only restructures pinned GPU test reference bundles and harnesses by variant; it makes no changes to the production dispatcher, candidate registry, routing, or kernel selectors.
- §E: Re-review against 01e4921 (rebase onto develop). All PR-owned changes under reference_common, conv_reference, sdpa_reference, and platform/cmake maintain structure and hygiene invariants.
- §F: Re-evaluated against head 01e4921 (which rebases/merges upstream develop changes); all policy.yml checks remain satisfied while local pre-commit and review thread states remain external to the diff.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The changes-since list names new or modified kernel-body emitter files under library/kernels/common/ and C++ instance files under platform/cpp/instances/common/, but their hunks are omitted, so I cannot verify whether every non-exempt Python family has a synchronized C++ twin or a documented known_gaps entry. |
| 2 | NEEDS-INFO | The changes-since list names core IR and lowering files in both platform/python/rocke/core/ and platform/cpp/core/, but the relevant hunks are omitted, so I cannot verify mirrored op-handler/lowering behavior. |
| 3 | NEEDS-INFO | The changes-since list names attention_unified.py and attention_unified_selectors.cpp plus related headers, but their hunks are omitted, so I cannot verify selector/spec/lowering/codegen parity. |
| 4 | NEEDS-INFO | The changed Python and C++ emitter hunks are omitted, so I cannot verify exact emit order, operand binding order, or canonical IR equality. |
| 5 | NEEDS-INFO | The changes-since list names representative and library golden JSON changes, but their contents and the corresponding IR-changing hunks are omitted, so I cannot tell whether each re-bless is intentional and appropriate. |
| 6 | NEEDS-INFO | The diff shown does not provide the cross-engine byte-identity gate result or identify whether golden changes were only Python-drift goldens versus check_byte_identity coverage. |
| 7 | NEEDS-INFO | The hot-path kernel and lowering hunks that would show whether new logic is guarded or unconditional are omitted. |
| 8 | NEEDS-INFO | The current diff includes C++ engine/instance/lowering file changes in the changes-since list, but no check_byte_identity result or complete C++ hunks are provided to review by hand. |
| 9 | NEEDS-INFO | The changes-since list names parity mini-pair files and shared lowering/op-handler files, but the relevant hunks and test registration details are omitted, so I cannot verify the required cheap proof exists for each shared fix or arch divergence. |
| 10 | NEEDS-INFO | The changes-since list names new feature/kernel work and golden JSON changes, but the omitted hunks do not show which flag-on IR paths, LLVM flavors, and architectures are covered by golden hash entries. |
| 11 | NEEDS-INFO | Core IR, opcode documentation, C++ IR headers, verifier, and lowerers are listed as changed, but their hunks are omitted, so I cannot determine whether a new IR op was added or whether every required touchpoint was updated. |
| 12 | NEEDS-INFO | platform/python/rocke/core/backend.py is listed as changed but its hunk is omitted, so I cannot verify ROCKE_BACKEND=both behavior or that CPP_UNPORTED_ARCHES remains empty. |
| 13 | NEEDS-INFO | The changes-since list names build-id, LLVM profile, lowering, and golden changes, but the emitted .ll generation hunks and gate environment are omitted, so I cannot verify absence of stamps or debug locations. |
| 14 | NEEDS-INFO | The changes-since list names classic helper changes in both Python and C++ helper files, but most helper hunks are omitted, so I cannot verify that every changed classic helper updates its C++ twin and header consistently. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Changes since the previous review include omitted rocKE kernel, dispatch, platform, and provider plan-builder hunks that may add new rejection gates; the full hunks are needed to verify every new rejection is classified as hardware limit versus implementation shortcut. |
| 2 | NEEDS-INFO | Changes since the previous review include omitted spec/validation and plan-builder hunks, so the full preflight and spec-constructor changes are needed to compare rejection scopes per architecture. |
| 3 | NEEDS-INFO | Changes since the previous review include omitted provider plan-builder and dispatch/routing hunks, so the full fallback-routing changes and their tests are needed to verify real model-zoo shapes are not silently demoted. |
| 4 | FAIL | The prior NEEDS-INFO is not resolved: the source-mode worker request is a trust boundary, but a missing meaningful variant field is normalized with request.get("variant"), which rewrites a malformed source request into the architecture's default/sole variant and runs that adapter instead of rejecting (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46). |
| 5 | NEEDS-INFO | The visible reference-variant gates are tested, but omitted kernel/dispatch/platform hunks in the changes since the previous review may open additional configurations; the full hunks are needed to verify each newly opened gate scope is tested or scoped down. |
| 6 | NEEDS-INFO | Changes since the previous review include omitted kernel/spec validation files that may touch is_valid_spec or supports_*; the full hunks are needed to verify those predicates remain functional-only and do not encode policy or coverage opinions. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Re-review resolves the previous NEEDS-INFO: repeated per-operation architecture/variant lookup is factored into the shared AdapterRegistry helper in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and both operation packages delegate to it in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:12 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:12. — no diff line supports this verdict |
| 2 | N/A | This PR refactors pinned GPU reference test layout and staging, not kernel build_<family>(spec) builders or emitted IR. |
| 3 | PASS | Variant names are operation/algorithm family names such as implicit_gemm_fwd in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4 and dense in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:4, and CMake rejects invalid or gfx-prefixed variant names in… |
| 4 | PASS | Arch-independent registry and lookup logic lives in the common helper dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, while processor packages now only name the enrolled processor, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/__init__.py:9, and algorithm-specific adapters live under variant subpackages such as… |
| 5 | PASS | The diff fan-out matches the inherent reference-key widening across tests, bundle paths, CMake staging, and docs: default_bundle_path gains a variant component in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, CMake installs variant payloads in dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:218, and TESTING documents the new installed path in… |
| 6 | PASS | The change is factored through the shared AdapterRegistry abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22 before operation packages bind get_adapter, variants, and baseline_lock to that abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:15. |
| 7 | PASS | Re-review resolves the previous NEEDS-INFO: the bundle move preserves the DVC object hash and size while changing only the archive path, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:2 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:5, and the baseline lock retains manifest_sha256 in… |
| 8 | PASS | New functionality extends the existing shared reference tooling rather than reimplementing separate operation tools: artifact.py adds the published command in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:316, and CLI cohort/variant handling is added to ReferenceOperation in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:170. |
| 9 | PASS | The variant-source/replay split is treated as a class across sibling workers: convolution dispatches source mode through get_adapter in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:45, and SDPA applies the same source/replay guard in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:64; CMake also rejects stale pre-variant locks and pointers repo-wide in… |
| 10 | PASS | The widened shared key (operation, architecture, variant) is propagated through option parsing in dnn-providers/hip-kernel-provider/rocke/library/tests/conftest.py:55, path construction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, pytest parametrization in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:178, CMake staging in… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892 reads the environment |
| 2 | N/A | This PR does not add or modify any dispatcher KernelCandidate or Capability declarations. |
| 3 | N/A | This PR does not add, modify, or call any dispatcher candidate _supports or admits predicates. |
| 4 | N/A | This PR does not touch dispatcher selectors or spec builders on the selection path. |
| 5 | N/A | This PR does not modify any KernelSpec identity, kernel_name, or cache-key logic. |
| 6 | N/A | This PR does not modify cohort routing, spec builders, launch metadata, or cache keys in the dispatcher. |
| 7 | N/A | This PR does not add or modify dispatcher heuristics, hand-pinned overrides, or resolvers. |
| 8 | N/A | This PR does not define or modify any dispatcher routing cohorts or spec override predicates. |
| 9 | N/A | This PR does not register any new kernel or architecture in the dispatcher registries. |
| 10 | N/A | This PR does not introduce any new KernelSpec fields or features. |
| 11 | N/A | This PR does not register, promote, or change precedence or opt-in status for any dispatcher candidates. |
| 12 | N/A | This PR only restructures pinned GPU test reference harnesses and does not add, rename, or reprioritize any dispatcher candidates. |
| 13 | N/A | This PR does not modify CandidateRegistry.select or add any dispatcher wrappers or entry points. |
| 14 | N/A | This PR does not touch kernel compilation caches, routing logs, tuning records, or kernarg layouts. |
| 15 | N/A | This PR does not add or modify any dispatcher candidates or arch modules in the production dispatch registry. |
| 16 | N/A | This PR does not introduce a new operator family in the dispatcher. |
| 17 | N/A | This PR does not add or modify dispatcher candidates or routing rules. |
| 18 | N/A | This PR does not modify CandidateRegistry settings, binding requirements, or exempt sets. |
| 19 | N/A | This PR makes no claims about unified attention auto routing or run_unified_attention_torch dispatch. |
| 20 | N/A | This PR makes no changes to kernel-level selectors, spec builders, or the Python registry/tuning layer. |
| 21 | N/A | This PR does not fix a kernel selection or emitter issue; it reorganizes test reference bundles by variant. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | library/tests imports rocke (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:57), while platform/cmake/InstallGpuReferences.cmake invokes artifact.py via an isolated subprocess (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:208) without importing library into platform. |
| 2 | PASS | Device queries in the reference test infrastructure use from rocke.runtime.hip_module import get_device_target_id (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:59) rather than torch. |
| 3 | PASS | Torch remains optional: dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/check_torch_reference.py:13 gates with pytest.importorskip("torch") and pinned reference tests verify assert not report["torch_imported"] (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:210). |
| 4 | N/A | This PR only modifies files under library/tests/, platform/, and TESTING.md, not production library/{kernels,dispatch,builders,benchmarks} layering or KNOWN_VIOLATIONS/KNOWN_BAD lists. |
| 5 | PASS | Platform CMake rules use relative directory variables (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:20) and Python helpers use tempfile and pathlib (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:18). |
| 6 | N/A | This PR does not touch emitter trees (core/, helpers/, instances/, library/kernels/). |
| 7 | PASS | Testing and workflow documentation are updated alongside the reference layout changes (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/docs/conv-test-reference.md:13, dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:321). |
| 8 | PASS | No Jira/tracking IDs, reviewer handles, or PR numbers appear in code or comments in the diff (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:1). |
| 9 | PASS | All PR-modified files are scoped to splitting pinned GPU reference bundles by operation, architecture, and variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | The PR was rebased onto current develop and explicitly guards against concurrent branches restoring pre-variant locks or DVC pointers at legacy paths (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107). |
| 11 | PASS | Only deliverable test harness code, locks, DVC pointers, CMake rules, and required reference documentation are committed (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22). |
| 12 | PASS | All enrolled architectures (gfx942, gfx950, gfx1151, gfx1200, gfx1201) are migrated to the variant layout (dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:471, dnn-providers/hip-kernel-provider/rocke/platform/cmake/PublishedGpuReferences.cmake:11). |
| 13 | NEEDS-INFO | Previously NEEDS-INFO due to diff budget truncation; cli.py, test_reference_common.py, test_gpu_references.py, and docs/*.md remain truncated or omitted in the prompt, although platform/cmake/InstallGpuReferences.cmake:61 and platform/cmake/InstallGpuReferences.cmake:107 enforce that no pre-variant locks or DVC pointers remain. |
| 14 | PASS | After rebasing onto develop, moved adapter files preserve upstream comments, historical API compatibility checks, and docstrings (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:43). |
| 15 | NEEDS-INFO | Previously NEEDS-INFO due to diff truncation; all visible new variant parameters in artifact.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143), registry.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65), and the visible portion of cli.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349) are read and used, but the tail of cli.py and test_gpu_references.py are truncated. |
| 16 | N/A | This item is a meta-instruction for review grading rather than a property of the pull request diff. |
| 17 | PASS | No performance numbers or superiority claims appear in the PR description, docs, or code (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/brockhargreaves-amd/pinned-reference-variants matches users/<user>/… (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
| 2 | PASS | title test(rocke): split pinned GPU references by operation, architecture, and variant matches type(scope): summary and is 80 characters |
| 3 | PASS | description is at least 30 characters and contains an issue reference |
| 4 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py is in the diff |
| 5 | PASS | no key, certificate, or secret filename in the diff |
| 6 | NEEDS-INFO | The new commits since eee5958 do not include local pre-commit execution logs, which are not visible from the diff alone. |
| 7 | NEEDS-INFO | Bot and Copilot review threads and their resolution/ticket status are not included in the prompt or diff. |
| 8 | PASS | Base branch is develop (and body notes Stacked on #13651), and the branch, title, body, tests, and files satisfy policy.yml checks (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312). |
| 9 | PASS | The PR keeps the full 10-commit migration and test suite together without artificial splitting for a size cap (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | Tests added in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:341 exercise real variant discovery, cohort digesting, and CMake staging behavior rather than throwaway checks. |
| 11 | PASS | The fetched PR metadata shows the populated Title and full Motivation/Technical Details/Test Plan/Test Result Body are present (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No shown change uses byte-identity or parity as a correctness proof. |
| 2 | NEEDS-INFO | Current commits introduce or wire kernel work, e.g. new convolution WMMA specs are imported in dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/direct_kernel_sweep.py:430, but the diff does not include current-head on-silicon numeric verification evidence with the required oracle/tolerance coverage. |
| 3 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: missing current-head evidence that the rocKE root tools/run_checks.py sweep, the extra llvm23 byte-identity check for emission changes, and the relevant pytest/golden/IR gates were run or proven unchanged versus develop. |
| 4 | NEEDS-INFO | The visible reference-variant behavior has tests, but the current-head changes since the previous review include many omitted behavior and test hunks, so it is missing enough diff to verify every new behavior has a fail-before test rather than only substring or whitespace coverage. |
| 5 | NEEDS-INFO | The visible reference tests exercise shipped paths, but the current-head changes since the previous review include many omitted tests and internal surfaces, so it is missing enough diff to verify every new test is falsifiable and reachable through the shipped path or explicitly scoped as defense-in-depth. |
| 6 | NEEDS-INFO | Previous NEEDS-INFO is not fully resolved: multiple documentation and code-comment hunks are still omitted, including reference_common docs, sdpa docs, platform/tests/test_gpu_references.py, and large current-head platform/kernel/performance hunks, so load-bearing numeric and structural prose cannot be fully mapped to checks. |
| 7 | N/A | No shown design doc introduces a new fp32 parity-reference oracle policy to review. |
| 8 | N/A | No shown rocKE harness change adds or modifies an fp8-to-fp16 dtype fallback. |
| 9 | NEEDS-INFO | The current-head changes include omitted resource/optimization-related hunks, so it is missing enough diff to verify whether any load-bearing LDS or occupancy numbers are tool-sourced rather than hand-derived. |
| 10 | N/A | No shown rocKE diff makes a performance-result claim requiring device, ratio, shipped-script, and parity provenance validation. |
| 11 | NEEDS-INFO | Current commits introduce or wire optimized kernels, e.g. dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/direct_kernel_sweep.py:443 adds new WMMA spec classes to launch planning, but the relevant routing/selector and measurement-path hunks are omitted, so reachability on the shipped surface cannot be verified. |
| 12 | NEEDS-INFO | Current-head changes include omitted Python/C++ platform, parity, and lowering hunks, so the two-engine or cross-layer gap cannot be graded by grepping callers and confirming which engine builds the changed path. |
| 13 | N/A | No shown change recommends a mangled LLVM intrinsic symbol or overload class that would require checking a .td definition. |
| 14 | N/A | No shown rocKE diff classifies a performance bottleneck using static analysis or dynamic counters. |
| 15 | N/A | No wrong-branch risk is evident from the shown rocKE diff content for this checklist item. |
| 16 | NEEDS-INFO | Resolves the previous NEEDS-INFO: the shared registry is defined in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:21, and operation architecture packages delegate lookups to it in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/init.py:12 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/init.py:12. — no diff line supports this verdict |
| 17 | NEEDS-INFO | The visible artifact CLI has end-to-end subprocess coverage, but current-head changes since the previous review add or modify multiple shipped scripts in omitted hunks, so it is missing enough diff to verify every shipped script was run end-to-end rather than only parsed. |
| 18 | PASS | Variant and cohort guards are wired into bundle validation and packaging (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:136, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:163), and constructed negative tests prove they fire for mismatched variants and mixed cohort locks (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:398… |
| 19 | PASS | The fixtures explicitly create the multi-variant, unpublished, missing-bundle, candidate-bundle, and selection states needed for the new gates (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1269, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1347). |
| 20 | N/A | This PR does not add or modify a detector banning performance or numerical claims. |
| 21 | N/A | This PR refactors pinned reference bundle layout and verification wiring, not performance or statistical measurement code. |
| 22 | PASS | The pinned harness fails selected unpublished or empty published-variant selections instead of passing without running (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:78), and CMake fails missing published archives during configuration (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:191). |
| 23 | N/A | This PR does not make performance speedup or routing A/B claims. |
| 24 | N/A | This PR does not re-bless IR goldens or use a golden re-bless as a correctness claim. |
| 25 | N/A | This PR does not contain a codegen micro-optimization. |
| 26 | N/A | This PR does not benchmark kernels or report performance measured on another target base. |
| 27 | N/A | This PR does not report an A/B result for a kernel optimization knob. |
| 28 | N/A | This PR does not report develop-to-PR performance deltas or counterfactual forced-off arms. |
| 29 | N/A | This PR does not swap guarded math primitives in a kernel. |
| 30 | N/A | This PR does not make no-spill, VGPR, scratch, or code-object resource claims. |
| 31 | N/A | This PR does not claim to fix a shipped kernel performance defect or bank conflict. |
| 32 | N/A | This PR does not report measured performance or hardware resource figures requiring provenance. |
| 33 | PASS | The changed variant resolver is explicitly enumerated by tests for enrolled, missing, ambiguous, override, and sole-variant selections (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1269), and downstream fixture outcomes are tabled across publication, bundle, option, and variant states (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1347). |
| 34 | N/A | This PR does not add or modify an opt-in kernel output flag. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only reorganizes test reference bundles and CMake/Python test harness infrastructure without touching kernel specs, SRDs, or launch validation. |
| 2 | N/A | This PR does not add or modify paged/indirection kernels or device table lookups. |
| 3 | N/A | This PR does not add or modify any device kernel code or compile-time tile/page divisors. |
| 4 | N/A | This PR does not touch device-side masks, bounds, or runtime sequence-length inputs. |
| 5 | N/A | This PR does not introduce or modify kernel launch inputs feeding device-side bounds or masks. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refactors test reference bundle layout by variant and does not perform kernel routing or candidate space measurement. |
| 2 | PASS | Invalid or half-wired states are loudly rejected with actionable error messages and recovery commands rather than silently ignored or cryptically failing (e.g., dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:39-135 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143-173). |
| 3 | N/A | This PR splits existing pinned reference cohorts by variant without altering test case generation or hypothesis property tests. |
| 4 | N/A | This PR does not generate performance tables or benchmark reports comparing shipped vs achievable kernel selections. |
| 5 | PASS | Pinned reference bundles are explicitly tracked as frozen snapshots via baseline_lock.json (recording baseline_revision, cohort_sha256, and manifest_sha256) with an explicit qualification/regen CLI path and status/diff drift checks (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349-495, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:1261-1317). |
| 6 | N/A | This PR does not add or modify any DSL ops, hardware instructions, or lowering primitives. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The modified Markdown visible in the diff (TESTING.md, conv-test-reference.md, platform/tests/README.md) contains no mathematical equations or matrix products to type-check. |
| 2 | N/A | The PR does not introduce mathematical formula notation tables or multi-domain quantities such as decay versus log-decay. |
| 3 | N/A | The visible Markdown documents GPU reference packaging and test workflows rather than restating a kernel algorithm derivation owned by another design document. |
| 4 | NEEDS-INFO | Not resolved from the previous NEEDS-INFO: the diff still omits the full Markdown hunks for dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/README.md, gpu-ci-pinned-rocke-test-reference-plan.md, gpu-reference-workflow.md, quickstart.md, and sdpa_reference/docs/sdpa-test-reference.md, so novelty claims in those files cannot be verified. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/TESTING.md:949-950 and dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:324-325 state the CTest timeout as a per-variant aggregate (five minutes per installed variant / five minutes per variant). |
| 6 | N/A | This PR reorganizes pinned test reference bundles and does not add model config.json tables, tuned dispatch band edges, or kernel constants requiring deployment provenance. |
| 7 | N/A | No new Markdown design or algorithm document file is added in the PR file list; existing test documentation files are updated in place. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any tuned dispatch candidates or KnobSpace subclasses. |
| 2 | N/A | This PR does not add or modify any kernel spec fields or KnobAxis declarations. |
| 3 | N/A | This PR does not change tuning_id, config_key, knob canonicalization rules, or tuned spec defaults. |
| 4 | N/A | This PR does not touch KnobSpace.defaults() or KnobSpace.recorded(). |
| 5 | N/A | This PR does not modify knob dropping, refusal, inertness, or base_value() rules. |
| 6 | N/A | This PR does not modify KnobSpace.validate or policy-free tuning spec construction. |
| 7 | N/A | This PR does not touch KnobSpace.base(req) or request normalization for tuning. |
| 8 | N/A | This PR does not change normalize_knobs or knob value coercion. |
| 9 | N/A | This PR modifies GPU test reference bundle replay, not dispatch tuning_id/spec_hash benchmark replay. |
| 10 | N/A | This PR does not touch KnobSpace.find or tuning_id resolution in dispatch. |
| 11 | N/A | This PR does not touch sweep entry points, pin_to_spec, or sweep harnesses. |
| 12 | N/A | This PR does not change production stacks, accept_default, or base specs for auto tuning selection. |
| 13 | N/A | This PR does not touch benchmark or sweep harnesses or make claims about auto selection. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR reorganizes pinned GPU test reference bundles by variant and does not add or modify any gfx hardware target or ISA backend. |
| 2 | N/A | This PR does not add or modify any MMA op_id or layout map. |
| 3 | N/A | This PR does not touch MMA selection, MfmaAtom catalogs, or kernel lowering. |
| 4 | N/A | This PR only relocates existing test adapter code and does not alter ArchTarget.wave_size or lane math. |
| 5 | N/A | This PR does not add or modify arch-specific kernel recipes or tile loaders. |
| 6 | N/A | This PR does not modify pipeline policies, ArchTarget, or rocke/ imports. |
| 7 | N/A | This PR does not add or modify logical dtypes, bit packing, instruction fragments, or FP8 dialects. |
| 8 | N/A | This PR does not touch scaled-MMA contracts or WMMA/MFMA scaled emission. |
| 9 | N/A | This PR does not introduce a new architecture target. |
| 10 | N/A | This PR does not modify intrinsic arch-domain artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The current diff lists changes to IR/control-flow and kernel-emitting files, but the relevant hunks for Value truthiness, scf_if/scf_for result threading, and emitted runtime predicates are omitted; provide the full hunks for platform/python/rocke/core/ir.py, platform/cpp/core/ir/ir_flow.cpp, and affected kernel builder files. |
| 2 | NEEDS-INFO | The current diff lists changes to target/architecture data and lowering-related files, but the relevant hunks for legalize_for_target behavior are omitted; provide the full hunks for the legalizer/arch-data changes to verify it only binds or rejects. |
| 3 | N/A | The visible diff and listed changed files do not show changes to ck.dsl.ir serialization, print_ir interchange behavior, wire versions, attribute tagging, SSA ids, or ABI parameter ordering. |
| 4 | NEEDS-INFO | The current diff lists changes to portable IR importer, recipe VM, and recording-builder files, but their hunks are omitted; provide the full hunks for platform/cpp/portable_ir/ir_import_json.cpp, platform/cpp/portable_ir/recipe_vm.cpp, and platform/python/rocke/portable_ir/src/recording_builder.py. |
| 5 | NEEDS-INFO | The current diff lists memory-lowering and kernel-body changes, but the relevant hunks are omitted; provide the full hunks needed to check AsyncTileLoader dword limits, LDS layout validation, destination pointer swizzling, and waitcnt placement. |
| 6 | NEEDS-INFO | The current diff lists changes to control, memory, cross-lane lowering, and kernel bodies, but the relevant hunks are omitted; provide the full hunks needed to check barriers in divergent control flow, invalid-pointer loads before select, and lane_xor constraints. |
| 7 | NEEDS-INFO | The current diff lists ISA backend and buffer/OOB-related changes, but the relevant hunks are omitted; provide the full hunks for buffer resource word 3 ownership and TemporalHint validation. |
| 8 | NEEDS-INFO | The current diff lists intrinsic-domain, lowering, LLVM profile, and backend-selection related changes, but the relevant hunks are omitted; provide the full hunks for intrinsic declarations/override tables and backend precedence changes. |
| 9 | NEEDS-INFO | The current diff lists changes under cpp/include/rocke and C++ binding/core files, but the relevant hunks are omitted; provide the full hunks for public C entry points and headers to verify stable extern "C" signatures and no fabricated HIP launch behavior. |
| 10 | NEEDS-INFO | The current diff lists LLVM-profile/lowering changes but omits the relevant hunks; provide the full hunks that emit or consume debug info to verify Debug Info Version flags and DWARF consumption behavior. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff does not add or modify helpers under platform/python/rocke/helpers/. |
| 2 | N/A | The PR diff does not author or modify kernels or their use of shared helper mechanisms. |
| 3 | N/A | The PR diff does not touch SoftwarePipeline or any kernel pipeline code. |
| 4 | N/A | The PR diff does not add or modify fused kernels, epilogue fusion logic, fusion validation, or fusion legalizers. |
| 5 | N/A | The PR diff does not introduce or change any public tiling API under helpers/tiling/. |
| 6 | N/A | The PR diff does not add or modify front-door or toolbox tiling helpers or any LayoutStyle implementation. |
| 7 | N/A | The PR diff does not touch tiling layout overrides, accumulator descriptors, or MMA validation. |
| 8 | N/A | The PR diff does not touch tiling memory operations, LDS allocation, global load/store emission, clipping, padding, or wide-load selection. |
| 9 | N/A | The PR diff does not use or modify N-D tensor axis roles, batch reductions, flow-stage relabeling, or AB-swap routing in tiling. |
| 10 | N/A | The PR diff does not modify tiling emit, LDS staging, MMA issue logic, or tiling fail-fast raises. |
| 11 | N/A | The PR diff does not touch MMA traits, TileMma, lds_swizzle, scheduling group helpers, or related tiling policies. |
| 12 | N/A | The PR diff makes no LDS bank-conflict or coalescing claims and does not modify related simulator or coalescing analysis tools. |
| 13 | N/A | The PR diff does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | The PR diff does not use tiling verbs, modify synchronization or barrier placement, or add a tiling pipeline variant. |
| 15 | N/A | The PR diff does not touch tiling allocation or bounds-checking logic for transformed address ranges. |
| 16 | N/A | The PR diff does not use or modify optimize_layout, recommend, or evaluate_transform. |
| 17 | N/A | The PR diff does not touch the tiling recorder, round-trip analysis, witness flags, or visualization modules. |
| 18 | N/A | The PR diff does not touch interleaved layouts, tiling demo kernels, dispatch registration of tiling demos, or tiling numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not touch core/verify.py, cpp/core/verify.cpp, or IR address spaces/attributes. |
| 2 | N/A | This PR does not modify scf.for, region structure, or IR block emission. |
| 3 | N/A | This PR does not modify IRBuilder, LLVM intrinsic declaration tables, or metadata emission. |
| 4 | N/A | This PR does not modify IR types, SmemType, or IR parsers. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in Python or C++ passes. |
| 7 | N/A | This PR does not modify kernel attributes, COMGR options, or cluster dimension checks. |
| 8 | N/A | This PR does not touch CodegenPolicy, SchedulerStrategy, or kernel compilation options. |
| 9 | N/A | This PR does not modify waitcnt encoding or barrier lowering. |
| 10 | N/A | This PR does not touch LLVM lowerers or parameter attribute/type lowering. |
| 11 | N/A | This PR does not touch ArchTarget or target catalog resolution. |
| 12 | N/A | This PR does not touch vector/LDS width caps or codegen arch defaults. |
| 13 | N/A | This PR does not touch public C headers under cpp/include/rocke/ or ABI versions. |
| 14 | N/A | This PR does not touch C++ engine sources, headers, bindings, or C++ compile flags in platform/CMakeLists.txt. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or alternative lowerers. |
| 16 | N/A | This PR does not touch known_gaps.md or C++ library/builder capabilities. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Missing hunks for the changed builder/instance files and the updated builder signature contract tests are needed to verify that all builders still take exactly (spec, *, arch) and that REQUIRED_FIELDS was not changed incompatibly. |
| 2 | NEEDS-INFO | Missing hunks for the changed conv ABI files, instance_conv_abi.h, packing/argument code, and parity emitters are needed to verify kernarg order and ABI-version parity. |
| 3 | NEEDS-INFO | Missing hunks for the changed launch-geometry and host harness code are needed to verify GEMM/conv grid single-sourcing and that harnesses do not recompute grids incorrectly. |
| 4 | NEEDS-INFO | Missing hunks for the changed LDS/admission gate files and C++ validator files are needed to verify the gate charges emitted allocations and uses ArchTarget capacity/fits_lds with C++ parity. |
| 5 | NEEDS-INFO | Missing hunks for the changed WMMA/RDNA GEMM, direct-conv WMMA, GemmMultiD/MultiAbd, and validator/spec files are needed to verify narrow-path restrictions and wave-size handling. |
| 6 | NEEDS-INFO | Missing hunks for the changed split-K/Stream-K/workspace-related kernels, helpers, launchers, and zeroing tests are needed to verify caller-zeroed workspace sizing and sentinel-zeroing coverage. |
| 7 | NEEDS-INFO | Missing hunks for SUPPORT_MATRIX.md, Stream-K sweep/launch code, grouped GEMM, and persistent/split-K admission guards are needed to verify known-rejected configurations remain rejected. |
| 8 | NEEDS-INFO | Missing hunks for the changed quantization, MX scale, MX GEMM, and decode helper files are needed to verify use of shared quant helpers, MX constraints, and edge decode behavior. |
| 9 | NEEDS-INFO | Missing hunks for the changed numeric emulation/decode code and edge-case tests are needed to verify finite overflow, Inf, NaN, denormal, and rounding-midpoint coverage in both engines. |
| 10 | N/A | This diff changes pinned GPU reference bundle metadata and descriptor-packaging plumbing, not rocKE runtime manifests under helpers/manifest.py or ck.dsl.example.manifest/v1. |
| 11 | NEEDS-INFO | Missing hunks for the changed runtime launcher, hip_module, stall_gate, and timing code are needed to verify buffer lifetime, stream synchronization, fences, WorkspacePool use, and HIP-event timing. |
| 12 | NEEDS-INFO | Missing hunks for the changed runtime/comgr.py and runtime/hip_module.py files are needed to verify COMGR/HIP loading does not map a second LLVM or import torch only to find ROCm libraries. |
| 13 | NEEDS-INFO | Missing hunks for the changed host-side launch math, kernel-cache, split-K, Stream-K, conv, and admission-guard code plus the corresponding test-plan documentation are needed to verify every consumer's numeric tests cover the refactor. |
| 14 | NEEDS-INFO | Missing hunks for the changed examples and any expected.json/performance-bound files are needed to verify correctness gates are preserved and no disallowed performance lower bound or misplaced kernel logic was added. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The diffs for dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt (+7 -2 lines) and dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py (+485 -80 lines) are still omitted to fit the model budget after the rebase, so CTest registration and installed test changes cannot be verified. |
| 2 | N/A | This PR refactors the GPU reference test bundle layout and does not add a new kernel family or variant to the platform IR or builders. |
| 3 | PASS | Tests and documentation grade the reference harness accurately, enforcing failure controls for perturbed outputs and missing launches in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:168-276 and noting subset verification caveats in dnn-providers/hip-kernel-provider/rocke/TESTING.md:154-156. |
| 4 | N/A | This PR does not re-bless IR goldens or widen the valid spec space in a lowerer or builder. |
| 5 | PASS | Tests remain portable across CPU/GPU and OS environments, skipping cleanly when hardware or bundles are absent (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:76-160) and querying the active device processor dynamically (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pytest_support.py:80-115). |
| 6 | PASS | The harness refactor preserves per-configuration semantics, scaling timeouts per installed variant (dnn-providers/hip-kernel-provider/rocke/TESTING.md:949-950), keeping at most one slot's workers alive at a time (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:82-163), and preserving error classes (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:227-232). |
| 7 | N/A | This PR does not modify engine backend rejection contracts or ROCKE_BACKEND / ROCKE_CPP_STRICT fallback behavior. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only restructures pinned GPU test reference harnesses and bundles and changes no kernel body, algorithm, tiling strategy, pipeline variant, or performance lever. |
| 2 | N/A | No performance lever or architecture-specific optimization recipe is modified in this test-reference refactoring PR. |
| 3 | N/A | No optimization lever covered by the utilities skills is touched by this test-infrastructure PR. |
| 4 | N/A | The diff does not alter kernel codegen, lowering, or performance primitives, so static ISA and occupancy probe evidence does not apply. |
| 5 | N/A | No global memory accesses in GPU kernels are added or modified by this diff. |
| 6 | N/A | No LDS or MMA layouts are added or changed in this pull request. |
| 7 | N/A | The PR makes no kernel bottleneck claims and implements no performance fixes. |
| 8 | N/A | No waits, barriers, or scheduling hints in GPU kernels are changed. |
| 9 | N/A | This PR is not a performance regression fix and changes no shape-dependent performance lever. |
| 10 | N/A | No compiler codegen changes, assembly parity claims, or performance benchmarks are included in this diff. |
| 11 | N/A | This PR introduces no new GPU optimization lessons, probes, or analysis tools. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The diff does not add a public rocke package symbol, spec/builder/grid helper, compile_kernel/KernelLauncher/time_launches parameter, run_manifest option, or rocke_* C entry point. |
| 2 | NEEDS-INFO | The previous NEEDS-INFO is resolved: existing pinned-reference cohort guards are updated to use variant adapters without dropping coverage at dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py:406 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:64. — no diff line supports this verdict |
| 3 | N/A | No public rocke package or C engine API is removed or renamed; the moved reference adapters are library test-harness internals. |
| 4 | N/A | No __all__ list in rocke, rocke.core, rocke.runtime, rocke.analysis, or rocke.helpers.tiling is edited by this diff. |
| 5 | PASS | The updated tests assert contracts and failure behavior, including variant mismatch rejection at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:398 and cohort digest binding at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:537. |
| 6 | PASS | CPU-checkable reference contracts are tested in host pytest such as the standard-library published-listing test at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:309, while launch/numeric pinned checks are explicitly GPU-marked at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:184. |
| 7 | N/A | The diff does not add or modify a platform API with a C++ twin or a C ABI binding. |
| 8 | NEEDS-INFO | Missing diff-citable evidence that the README entry points from rocke/platform/ were run at the current head, including tests/test_rocke.py and tests/run_all.py, and any applicable manifest/runtime verification. |
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since 01e4921ab16f
ROCm/rocm-libraries#14164 · e49fb56d9f82 · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 2, MEDIUM 0, LOW 0, NEEDS-INFO 28
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (01e4921ab16f)
Resolved 0 · still open 2 · new 0
- ⏳ §B.4 still FAIL (HIGH): In
dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:65, replacerequest.get("variant")withrequest["variant"]and requireisinstance(request["variant"], str)inget_adapterorrunfor source-mode requests. - ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
Findings
- HIGH §B ·
dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46· Indnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:65, replacerequest.get("variant")withrequest["variant"]and requireisinstance(request["variant"], str)inget_adapterorrunfor source-mode requests. - HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892· Put the knob on the KernelSpec and delete the environment read.
Needs info
- §C.1 Re-review resolves the previous NEEDS-INFO: repeated per-operation architecture/variant lookup is factored into the shared
AdapterRegistryhelper indnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and both operation packages delegate to it indnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict - §E.13 The rebase since the previous review did not modify the PR's files, and
cli.py(truncated at line 1445),test_reference_common.py(truncated at line 1676),test_gpu_references.py, andreference_common/docs/*.mdare still truncated or omitted in the prompt, althoughdnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61anddnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107enforce that no pre-variant locks or DVC pointers remain. - §E.15 The rebase since the previous review did not modify the PR's files; all visible new
variantparameters inartifact.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read atdnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143),registry.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65), and the visible portion ofcli.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349) are read and used, but the tail ofcli.py(after line 1445) andtest_gpu_references.pyremain truncated/omitted. - §F.6 The new commits since 01e4921 do not include local
pre-commitexecution logs, which cannot be verified from the diff alone. - §F.7 The changes since 01e4921 do not include GitHub review thread metadata, so bot/Copilot thread resolution and deferred ticket IDs cannot be verified.
- §G.3 Previous NEEDS-INFO is not resolved: although the PR no longer modifies kernel emission, the PR test plan and diff still do not provide evidence that the rocKE root tools/run_checks.py gate sweep was run or proven unchanged versus develop.
- §G.4 The rebase removed the unrelated kernel files, and visible new behaviors have fail-before tests (e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312), but the tail of dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py, the tail of dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py, and all of dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py are omitted from the diff.
- §G.5 Visible tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:338 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:482 are falsifiable and exercise shipped entry points, but the truncated portion of dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py and the omitted dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py prevent verifying all added tests.
Section notes
- §A: The previous NEEDS-INFO grades were caused by changes-since history from rebasing onto develop; against the current PR diff (
Files:list), this PR only reorganizes pinned GPU reference test harnesses, locks, DVC pointers, CMake staging, and docs, and does not touch any kernel emitters, selectors, core IR/lowering, helpers, or IR goldens. - §D: This PR only restructures pinned GPU test reference harnesses by (operation, architecture, variant) and does not modify production dispatcher registration, routing, or kernel selection.
- §E: The branch was rebased onto develop; the PR-specific diff continues to pass all verifiable checks, while items 13 and 15 remain NEEDS-INFO due to prompt budget truncation of cli.py, test_reference_common.py, test_gpu_references.py, and docs/*.md.
- §G: Re-reviewed against e49fb56 after rebase onto develop; the PR diff now contains only pinned GPU reference variant harness changes.
- §G: Re-review includes a truncated changes-since-previous section with many omitted rocKE benchmark, dispatch, kernel, golden, and LSE hunks; items depending on those missing hunks are marked NEEDS-INFO.
- §H: No device-side masks, bounds, indirection tables, or kernel implementations are added or modified by this PR; all changes in the PR diff are confined to pinned GPU reference test harness layout, CMake staging, DVC pointers, and documentation.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | Resolved from previous NEEDS-INFO: the PR diff against base (dnn-providers/hip-kernel-provider/rocke/library/tests/... and platform/cmake/InstallGpuReferences.cmake) touches only pinned GPU reference test… |
| 2 | N/A | Resolved from previous NEEDS-INFO: no core/lower_* or core/ir files are modified in this PR's file list. |
| 3 | N/A | Resolved from previous NEEDS-INFO: this PR does not modify any kernel selectors, spec builders, lowering, or codegen knobs. |
| 4 | N/A | Resolved from previous NEEDS-INFO: this PR modifies no Python or C++ IR emitters. |
| 5 | N/A | Resolved from previous NEEDS-INFO: no representative-IR or library IR golden files are touched in this PR (only test reference baseline_lock.json and .dvc files are moved/updated). |
| 6 | N/A | Resolved from previous NEEDS-INFO: this PR makes no IR or engine changes requiring parity or golden proof. |
| 7 | N/A | Resolved from previous NEEDS-INFO: no kernel or lowering logic is added or modified on any IR emission path. |
| 8 | N/A | Resolved from previous NEEDS-INFO: no C++ rocKE engine, lowering, helper, or instance code is touched in this PR. |
| 9 | N/A | Resolved from previous NEEDS-INFO: no shared op-handler fix or lowerer arch divergence is introduced in this PR. |
| 10 | N/A | Resolved from previous NEEDS-INFO: no new kernel or IR feature flag is introduced in this PR. |
| 11 | N/A | Resolved from previous NEEDS-INFO: no new IR op is added in this PR. |
| 12 | N/A | Resolved from previous NEEDS-INFO: core/backend.py and ROCKE_BACKEND handling are not touched by this PR. |
| 13 | N/A | Resolved from previous NEEDS-INFO: .ll emission and IR gate environments are not modified by this PR. |
| 14 | N/A | Resolved from previous NEEDS-INFO: no classic helper files under platform/python/rocke/helpers/ or platform/cpp/helpers/ are modified by this PR. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff Against base (develop) only modifies test reference infrastructure, CMake staging, and docs, and does not add any kernel or dispatcher rejection gates (the broader commit range since the previous review is… |
| 2 | N/A | This PR does not touch any kernel spec constructors or preflight gates. |
| 3 | N/A | This PR only reorganizes pinned GPU reference test bundles by variant and does not change kernel routing or fallback behavior. |
| 4 | FAIL | The previous FAIL is not resolved: in source mode, conv_reference/worker.py (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46) and sdpa_reference/worker.py (dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:65) still use request.get("variant"), which normalizes a malformed source request missing variant into None and causes get_adapter (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:72) to silently fall back to the architecture's sole variant instead of rejecting. |
| 5 | PASS | The previous NEEDS-INFO was caused by rebase commits from develop; in the actual PR diff, the new variant-selection, cohort-digest, and CMake reference-wiring gates (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:77) are covered by host tests… |
| 6 | N/A | This PR does not add or modify any is_valid_spec or supports_* predicates. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Re-review resolves the previous NEEDS-INFO: repeated per-operation architecture/variant lookup is factored into the shared AdapterRegistry helper in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and both operation packages delegate to it in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict |
| 2 | N/A | This PR refactors pinned GPU reference test layout and staging, not kernel build_<family>(spec) builders or emitted IR. |
| 3 | PASS | Variant names are operation/algorithm family names such as implicit_gemm_fwd in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4 and dense in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:4, and CMake rejects invalid or gfx-prefixed variant names in… |
| 4 | PASS | Arch-independent registry and lookup logic lives in the common helper dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, while processor packages now only name the enrolled processor, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/__init__.py:9, and algorithm-specific adapters live under variant subpackages such as… |
| 5 | PASS | The diff fan-out matches the inherent reference-key widening across tests, bundle paths, CMake staging, and docs: default_bundle_path gains a variant component in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, CMake installs variant payloads in dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:218, and TESTING documents the new installed path in… |
| 6 | PASS | The change is factored through the shared AdapterRegistry abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22 before operation packages bind get_adapter, variants, and baseline_lock to that abstraction in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:15. |
| 7 | PASS | The bundle move preserves the DVC object hash and size while changing only the archive path, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:2 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:5, and the baseline lock retains manifest_sha256 in… |
| 8 | PASS | New functionality extends the existing shared reference tooling rather than reimplementing separate operation tools: artifact.py adds the published command in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:316, and CLI cohort/variant handling is added to ReferenceOperation in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:170. |
| 9 | PASS | The variant-source/replay split is treated as a class across sibling workers: convolution dispatches source mode through get_adapter in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:45, and SDPA applies the same source/replay guard in dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:64; CMake also rejects stale pre-variant locks and pointers repo-wide in… |
| 10 | PASS | The widened shared key (operation, architecture, variant) is propagated through option parsing in dnn-providers/hip-kernel-provider/rocke/library/tests/conftest.py:55, path construction in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, pytest parametrization in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:178, CMake staging in… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892 reads the environment |
| 2 | N/A | This PR does not add or modify any dispatcher KernelCandidate or Capability declarations. |
| 3 | N/A | This PR does not add, modify, or call any dispatcher candidate _supports or admits predicates. |
| 4 | N/A | This PR does not touch dispatcher selectors or spec builders on the selection path. |
| 5 | N/A | This PR does not modify any KernelSpec identity, kernel_name, or cache-key logic. |
| 6 | N/A | This PR does not modify cohort routing, spec builders, launch metadata, or cache keys in the dispatcher. |
| 7 | N/A | This PR does not add or modify dispatcher heuristics, hand-pinned overrides, or resolvers. |
| 8 | N/A | This PR does not define or modify any dispatcher routing cohorts or spec override predicates. |
| 9 | N/A | This PR does not register any new kernel or architecture in the dispatcher registries. |
| 10 | N/A | This PR does not introduce any new KernelSpec fields or features. |
| 11 | N/A | This PR does not register, promote, or change precedence or opt-in status for any dispatcher candidates. |
| 12 | N/A | This PR only restructures pinned GPU test reference harnesses and does not add, rename, or reprioritize any dispatcher candidates. |
| 13 | N/A | This PR does not modify CandidateRegistry.select or add any dispatcher wrappers or entry points. |
| 14 | N/A | This PR does not touch kernel compilation caches, routing logs, tuning records, or kernarg layouts. |
| 15 | N/A | This PR does not add or modify any dispatcher candidates or arch modules in the production dispatch registry. |
| 16 | N/A | This PR does not introduce a new operator family in the dispatcher. |
| 17 | N/A | This PR does not add or modify dispatcher candidates or routing rules. |
| 18 | N/A | This PR does not modify CandidateRegistry settings, binding requirements, or exempt sets. |
| 19 | N/A | This PR makes no claims about unified attention auto routing or run_unified_attention_torch dispatch. |
| 20 | N/A | This PR makes no changes to kernel-level selectors, spec builders, or the Python registry/tuning layer. |
| 21 | N/A | This PR does not fix a kernel selection or emitter issue; it reorganizes test reference bundles by variant. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | library/tests imports rocke (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:57), while platform/cmake/InstallGpuReferences.cmake invokes artifact.py via an isolated subprocess (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:208) without importing library into platform. |
| 2 | PASS | Device queries in the reference test infrastructure use from rocke.runtime.hip_module import get_device_target_id (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:59) rather than torch. |
| 3 | PASS | Torch remains optional: dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/check_torch_reference.py:13 gates with pytest.importorskip("torch") and pinned reference tests verify assert not report["torch_imported"] (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:210). |
| 4 | N/A | This PR only modifies files under library/tests/, platform/, and TESTING.md, not production library/{kernels,dispatch,builders,benchmarks} layering or KNOWN_VIOLATIONS/KNOWN_BAD lists. |
| 5 | PASS | Platform CMake rules use relative directory variables (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:20) and Python helpers use tempfile and pathlib (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:18). |
| 6 | N/A | This PR does not touch emitter trees (core/, helpers/, instances/, library/kernels/). |
| 7 | PASS | Testing and workflow documentation are updated alongside the reference layout changes (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/docs/conv-test-reference.md:13, dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:338). |
| 8 | PASS | No Jira/tracking IDs, reviewer handles, or PR numbers appear in code or comments in the diff (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:1). |
| 9 | PASS | All PR-modified files are scoped to splitting pinned GPU reference bundles by operation, architecture, and variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | The PR was rebased onto current develop and explicitly guards against concurrent branches restoring pre-variant locks or DVC pointers at legacy paths (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107). |
| 11 | PASS | Only deliverable test harness code, locks, DVC pointers, CMake rules, and required reference documentation are committed (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22). |
| 12 | PASS | All enrolled architectures (gfx942, gfx950, gfx1151, gfx1200, gfx1201) are migrated to the variant layout (dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:483, dnn-providers/hip-kernel-provider/rocke/platform/cmake/PublishedGpuReferences.cmake:11). |
| 13 | NEEDS-INFO | The rebase since the previous review did not modify the PR's files, and cli.py (truncated at line 1445), test_reference_common.py (truncated at line 1676), test_gpu_references.py, and reference_common/docs/*.md are still truncated or omitted in the prompt, although dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61 and dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107 enforce that no pre-variant locks or DVC pointers remain. |
| 14 | PASS | After rebasing onto develop, moved adapter files preserve upstream comments, historical API compatibility checks, and docstrings (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:43). |
| 15 | NEEDS-INFO | The rebase since the previous review did not modify the PR's files; all visible new variant parameters in artifact.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143), registry.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65), and the visible portion of cli.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349) are read and used, but the tail of cli.py (after line 1445) and test_gpu_references.py remain truncated/omitted. |
| 16 | N/A | This item is a meta-instruction for review grading rather than a property of the pull request diff. |
| 17 | PASS | No performance numbers or superiority claims appear in the PR description, docs, or code (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/brockhargreaves-amd/pinned-reference-variants matches users/<user>/… (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
| 2 | PASS | title test(rocke): split pinned GPU references by operation, architecture, and variant matches type(scope): summary and is 80 characters |
| 3 | PASS | description is at least 30 characters and contains an issue reference |
| 4 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py is in the diff |
| 5 | PASS | no key, certificate, or secret filename in the diff |
| 6 | NEEDS-INFO | The new commits since 01e4921 do not include local pre-commit execution logs, which cannot be verified from the diff alone. |
| 7 | NEEDS-INFO | The changes since 01e4921 do not include GitHub review thread metadata, so bot/Copilot thread resolution and deferred ticket IDs cannot be verified. |
| 8 | PASS | Base branch is develop (body notes Stacked on #13651), and the branch prefix, Conventional Commit title, description with issue ref, unit tests, and absence of forbidden files satisfy policy.yml (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312). |
| 9 | PASS | The full variant-keyed reference migration and test suite remain together in one PR without artificial splitting for a size cap (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | Tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:353 exercise real variant discovery, cohort digesting, and CMake staging invariants rather than throwaway checks. |
| 11 | PASS | The fetched PR metadata shows the populated Title and complete Motivation/Technical Details/Test Plan/Test Result Body are intact (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No change in this PR uses byte-identity or parity as a correctness proof. |
| 2 | N/A | Resolves the previous NEEDS-INFO: after rebasing onto develop, this PR only reorganizes pinned GPU reference test harnesses and bundles (e.g.… |
| 3 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: although the PR no longer modifies kernel emission, the PR test plan and diff still do not provide evidence that the rocKE root tools/run_checks.py gate sweep was run or proven unchanged versus develop. |
| 4 | NEEDS-INFO | The rebase removed the unrelated kernel files, and visible new behaviors have fail-before tests (e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312), but the tail of dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py, the tail of dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py, and all of dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py are omitted from the diff. |
| 5 | NEEDS-INFO | Visible tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:338 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:482 are falsifiable and exercise shipped entry points, but the truncated portion of dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py and the omitted dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py prevent verifying all added tests. |
| 6 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: multiple documentation and test hunks in this PR are omitted from the diff, including dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/README.md, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/gpu-reference-workflow.md, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/quickstart.md, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/docs/sdpa-test-reference.md, and dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py. |
| 7 | N/A | This PR does not introduce a new fp32 parity-reference oracle policy in a design doc. |
| 8 | N/A | No harness change in this PR adds or modifies a dtype fallback such as fp8 to fp16. |
| 9 | N/A | Resolves the previous NEEDS-INFO: after rebasing onto develop, this PR contains only reference test harness changes and no LDS or occupancy numbers. |
| 10 | N/A | This PR makes no kernel performance claims requiring device, ratio, script, or parity provenance. |
| 11 | N/A | Resolves the previous NEEDS-INFO: after rebasing onto develop, this PR does not introduce or wire any optimized kernels or benchmark specs. |
| 12 | N/A | Resolves the previous NEEDS-INFO: after rebasing onto develop, this PR only touches the Python/CMake pinned reference test harness and introduces no two-engine or cross-layer kernel/platform changes. |
| 13 | N/A | This PR does not touch LLVM intrinsics or recommend mangled intrinsic symbols. |
| 14 | N/A | This PR does not classify any performance bottleneck. |
| 15 | N/A | No wrong-branch risk applies because the PR diff is provided directly from the head commit. |
| 16 | NEEDS-INFO | Resolves the previous NEEDS-INFO: the shared AdapterRegistry is defined in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22 and wired into both operation packages in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/init.py:12 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/init.py:12. — no diff line supports this verdict |
| 17 | PASS | Resolves the previous NEEDS-INFO: after rebasing onto develop, the only modified shipped CLI scripts are exercised end-to-end via subprocess in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:316 and CMake execute_process in dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:206. |
| 18 | PASS | Variant and cohort guards are wired into bundle authentication (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:136, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:157), and negative tests construct mismatched variant/cohort failures (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:398… |
| 19 | PASS | The fixtures now create multi-variant, ambiguous, override, sole-variant, published, unpublished, missing-bundle, and candidate-bundle states needed for the gates (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1269, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1593). |
| 20 | NEEDS-INFO | The changes-since-previous section lists omitted claim/support-checking script hunks, so the diff is missing the detector logic needed to determine whether any claim ban sweeps words rather than only digits. |
| 21 | NEEDS-INFO | The changes-since-previous section lists omitted rocKE benchmark and performance-harness hunks, so the diff is missing the measurement-code changes needed to verify accepted inputs, measured/modelled labeling, false-positive behavior, and tolerance semantics. |
| 22 | PASS | The selected pinned-reference path fails unpublished or empty selected variants instead of silently passing (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:76), and CMake fails missing published variant archives during staging (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:194). |
| 23 | NEEDS-INFO | The changes-since-previous section lists omitted benchmark, routing, dispatch, and documentation hunks, so the diff is missing the perf/routing claims needed to verify configuration labeling and oracle comparisons. |
| 24 | NEEDS-INFO | The changes-since-previous section lists many omitted rocKE golden JSON hunks, so the diff is missing the re-blessed hash entries and validation evidence needed to assess golden-scope GPU coverage. |
| 25 | NEEDS-INFO | The changes-since-previous section lists omitted kernel/codegen and benchmark hunks, so the diff is missing the per-kernel resource deltas and corpus A/B evidence needed to assess any codegen micro-optimization. |
| 26 | NEEDS-INFO | The changes-since-previous section lists omitted benchmark and performance-documentation hunks, so the diff is missing the measurement provenance needed to verify the benchmark worktree was on the PR target base. |
| 27 | NEEDS-INFO | The changes-since-previous section lists omitted benchmark, dispatch, and kernel-tuning hunks, so the diff is missing the A/B toggle details needed to verify only the shipped knob was toggled. |
| 28 | NEEDS-INFO | The changes-since-previous section lists omitted performance and routing hunks, so the diff is missing any develop-to-PR or counterfactual tables needed to verify zero-delta and forced-off-arm handling. |
| 29 | NEEDS-INFO | The changes-since-previous section lists omitted kernel math hunks, so the diff is missing the primitive-use sites needed to determine whether any guarded fast-primitive swap respects its precondition. |
| 30 | NEEDS-INFO | The changes-since-previous section lists omitted benchmark/resource-reporting hunks, so the diff is missing any no-spill or resource claims and code-object evidence needed for assessment. |
| 31 | NEEDS-INFO | The changes-since-previous section lists omitted performance-defect, dispatch, kernel, and benchmark hunks, so the diff is missing the shipped-baseline A/B evidence needed to verify any defect was real before the PR. |
| 32 | NEEDS-INFO | The changes-since-previous section lists omitted benchmark and measurement hunks, so the diff is missing the reproducible provenance and compile/run target confirmation for measured figures. |
| 33 | NEEDS-INFO | The shown variant resolver is differentially exercised across ambiguous, override, and sole-variant states (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1269), but the changes-since-previous section lists many omitted dispatch/resolver hunks, so the diff is missing the full set of changed pure decision functions and their differential tables. |
| 34 | NEEDS-INFO | The changes-since-previous section lists omitted LSE/output-flag and attention-output hunks, so the diff is missing the flag-on/off output comparisons needed to verify pre-existing outputs are unchanged. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only reorganizes test reference bundles and CMake/Python test harness infrastructure without touching kernel specs, SRDs, or launch validation. |
| 2 | N/A | This PR does not add or modify paged/indirection kernels or device table lookups. |
| 3 | N/A | This PR does not add or modify any device kernel code or compile-time tile/page divisors. |
| 4 | N/A | This PR does not touch device-side masks, bounds, or runtime sequence-length inputs. |
| 5 | N/A | This PR does not introduce or modify kernel launch inputs feeding device-side bounds or masks. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR refactors test reference bundle layout by variant and does not perform kernel routing or candidate space measurement. |
| 2 | PASS | Invalid or half-wired states are loudly rejected with actionable error messages and recovery commands rather than silently ignored or cryptically failing (e.g., dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:39-135 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143-173). |
| 3 | N/A | This PR splits existing pinned reference cohorts by variant without altering test case generation or hypothesis property tests. |
| 4 | N/A | This PR does not generate performance tables or benchmark reports comparing shipped vs achievable kernel selections. |
| 5 | PASS | Pinned reference bundles are explicitly tracked as frozen snapshots via baseline_lock.json (recording baseline_revision, cohort_sha256, and manifest_sha256) with an explicit qualification/regen CLI path and status/diff drift checks (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349-495, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:1261-1317). |
| 6 | N/A | This PR does not add or modify any DSL ops, hardware instructions, or lowering primitives. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The modified Markdown visible in the diff (TESTING.md, conv-test-reference.md, platform/tests/README.md) contains no mathematical equations or matrix products to type-check. |
| 2 | N/A | The PR does not introduce mathematical formula notation tables or multi-domain quantities such as decay versus log-decay. |
| 3 | N/A | The visible Markdown documents GPU reference packaging and test workflows rather than restating a kernel algorithm derivation owned by another design document. |
| 4 | NEEDS-INFO | Not resolved from the previous NEEDS-INFO: the rebase commit (01e4921..e49fb56) did not make the omitted PR Markdown hunks visible (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/README.md, gpu-ci-pinned-rocke-test-reference-plan.md, gpu-reference-workflow.md, quickstart.md, and sdpa_reference/docs/sdpa-test-reference.md remain omitted to fit the model budget), so novelty claims in those files still cannot be verified. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/TESTING.md:1010-1011 and dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:341-342 state the CTest timeout as a per-variant aggregate (five minutes per installed variant / five minutes per variant). |
| 6 | N/A | This PR reorganizes pinned test reference bundles and does not add model config.json tables, tuned dispatch band edges, or kernel constants requiring deployment provenance. |
| 7 | N/A | No new Markdown design or algorithm document file is added in the PR file list; existing test documentation files are updated in place. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not add or modify any tuned dispatch candidates or KnobSpace subclasses. |
| 2 | N/A | This PR does not add or modify any kernel spec fields or KnobAxis declarations. |
| 3 | N/A | This PR does not change tuning_id, config_key, knob canonicalization rules, or tuned spec defaults. |
| 4 | N/A | This PR does not touch KnobSpace.defaults() or KnobSpace.recorded(). |
| 5 | N/A | This PR does not modify knob dropping, refusal, inertness, or base_value() rules. |
| 6 | N/A | This PR does not modify KnobSpace.validate or policy-free tuning spec construction. |
| 7 | N/A | This PR does not touch KnobSpace.base(req) or request normalization for tuning. |
| 8 | N/A | This PR does not change normalize_knobs or knob value coercion. |
| 9 | N/A | This PR modifies GPU test reference bundle replay, not dispatch tuning_id/spec_hash benchmark replay. |
| 10 | N/A | This PR does not touch KnobSpace.find or tuning_id resolution in dispatch. |
| 11 | N/A | This PR does not touch sweep entry points, pin_to_spec, or sweep harnesses. |
| 12 | N/A | This PR does not change production stacks, accept_default, or base specs for auto tuning selection. |
| 13 | N/A | This PR does not touch benchmark or sweep harnesses or make claims about auto selection. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR reorganizes pinned GPU test reference bundles by variant and does not add or modify any gfx hardware target or ISA backend. |
| 2 | N/A | This PR does not add or modify any MMA op_id or layout map. |
| 3 | N/A | This PR does not touch MMA selection, MfmaAtom catalogs, or kernel lowering. |
| 4 | N/A | This PR only relocates existing test adapter code and does not alter ArchTarget.wave_size or lane math. |
| 5 | N/A | This PR does not add or modify arch-specific kernel recipes or tile loaders. |
| 6 | N/A | This PR does not modify pipeline policies, ArchTarget, or rocke/ imports. |
| 7 | N/A | This PR does not add or modify logical dtypes, bit packing, instruction fragments, or FP8 dialects. |
| 8 | N/A | This PR does not touch scaled-MMA contracts or WMMA/MFMA scaled emission. |
| 9 | N/A | This PR does not introduce a new architecture target. |
| 10 | N/A | This PR does not modify intrinsic arch-domain artifacts or LLVM flavor membership lists. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff only modifies test reference files under rocke/library/tests/ and rocke/platform/cmake/, with no changes to core/ IR, builders, or kernel… |
| 2 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not touch legalize_for_target, architecture data, or lowering files. |
| 3 | N/A | The PR diff does not touch ck.dsl.ir serialization, print_ir, wire versions, attribute tagging, SSA ids, or ABI parameter ordering. |
| 4 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not touch portable IR schemas, importers, recipe expansion/VM, or recording builders. |
| 5 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not touch AsyncTileLoader, LDS layouts, or kernel memory lowering. |
| 6 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not emit or modify kernel control flow, barriers, memory loads, or cross-lane shuffles. |
| 7 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not touch ISA backends, buffer resource word 3, or TemporalHint handling. |
| 8 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not add intrinsic declarations, modify LLVM profile tables, or change backend selection precedence. |
| 9 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not touch public C entry points, headers under cpp/include/rocke/, or C++ host launch behavior. |
| 10 | N/A | Resolved since the previous review by rebasing onto develop: the current PR diff does not emit or consume LLVM debug info (!dbg) or DWARF metadata. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff does not add or modify helpers under platform/python/rocke/helpers/. |
| 2 | N/A | The PR diff does not author or modify kernels or their use of shared helper mechanisms. |
| 3 | N/A | The PR diff does not touch SoftwarePipeline or any kernel pipeline code. |
| 4 | N/A | The PR diff does not add or modify fused kernels, epilogue fusion logic, fusion validation, or fusion legalizers. |
| 5 | N/A | The PR diff does not introduce or change any public tiling API under helpers/tiling/. |
| 6 | N/A | The PR diff does not add or modify front-door or toolbox tiling helpers or any LayoutStyle implementation. |
| 7 | N/A | The PR diff does not touch tiling layout overrides, accumulator descriptors, or MMA validation. |
| 8 | N/A | The PR diff does not touch tiling memory operations, LDS allocation, global load/store emission, clipping, padding, or wide-load selection. |
| 9 | N/A | The PR diff does not use or modify N-D tensor axis roles, batch reductions, flow-stage relabeling, or AB-swap routing in tiling. |
| 10 | N/A | The PR diff does not modify tiling emit, LDS staging, MMA issue logic, or tiling fail-fast raises. |
| 11 | N/A | The PR diff does not touch MMA traits, TileMma, lds_swizzle, scheduling group helpers, or related tiling policies. |
| 12 | N/A | The PR diff makes no LDS bank-conflict or coalescing claims and does not modify related simulator or coalescing analysis tools. |
| 13 | N/A | The PR diff does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | The PR diff does not use tiling verbs, modify synchronization or barrier placement, or add a tiling pipeline variant. |
| 15 | N/A | The PR diff does not touch tiling allocation or bounds-checking logic for transformed address ranges. |
| 16 | N/A | The PR diff does not use or modify optimize_layout, recommend, or evaluate_transform. |
| 17 | N/A | The PR diff does not touch the tiling recorder, round-trip analysis, witness flags, or visualization modules. |
| 18 | N/A | The PR diff does not touch interleaved layouts, tiling demo kernels, dispatch registration of tiling demos, or tiling numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not touch core/verify.py, cpp/core/verify.cpp, or IR address spaces/attributes. |
| 2 | N/A | This PR does not modify scf.for, region structure, or IR block emission. |
| 3 | N/A | This PR does not modify IRBuilder, LLVM intrinsic declaration tables, or metadata emission. |
| 4 | N/A | This PR does not modify IR types, SmemType, or IR parsers. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in Python or C++ passes. |
| 7 | N/A | This PR does not modify kernel attributes, COMGR options, or cluster dimension checks. |
| 8 | N/A | This PR does not touch CodegenPolicy, SchedulerStrategy, or kernel compilation options. |
| 9 | N/A | This PR does not modify waitcnt encoding or barrier lowering. |
| 10 | N/A | This PR does not touch LLVM lowerers or parameter attribute/type lowering. |
| 11 | N/A | This PR does not touch ArchTarget or target catalog resolution. |
| 12 | N/A | This PR does not touch vector/LDS width caps or codegen arch defaults. |
| 13 | N/A | This PR does not touch public C headers under cpp/include/rocke/ or ABI versions. |
| 14 | N/A | This PR does not touch C++ engine sources, headers, bindings, or C++ compile flags in platform/CMakeLists.txt. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or alternative lowerers. |
| 16 | N/A | This PR does not touch known_gaps.md or C++ library/builder capabilities. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The current PR diff (Files list and Diff) does not touch any kernel builders, specs, or REQUIRED_FIELDS; the earlier NEEDS-INFO came from un-rebased base changes. |
| 2 | N/A | The current PR diff does not touch any kernel signatures, conv_abi, instance_conv_abi.h, or runtime argument packing. |
| 3 | N/A | The current PR diff does not change any GEMM, conv, or block-scaled GEMM launch geometry or grid calculation logic (conv_reference/architectures/gfx942/implicit_gemm_fwd/init.py:131 is a pure file move). |
| 4 | N/A | The current PR diff does not touch any LDS capacity gates, admission checks, or C++ validators. |
| 5 | N/A | The current PR diff does not touch WMMA/RDNA GEMM, GemmMultiD/GemmMultiAbd, or wave-size spec validation. |
| 6 | N/A | The current PR diff does not touch Stream-K, MoE sorting, fused MoE, or split-K workspace sizing or zeroing. |
| 7 | N/A | The current PR diff does not touch Stream-K, persistent GEMM, GroupedGemmLauncher, or instances/SUPPORT_MATRIX.md. |
| 8 | N/A | The current PR diff does not touch quantization helpers, MX GEMM, or MX scale decode. |
| 9 | N/A | The current PR diff does not touch numeric emulation or decode paths in either engine. |
| 10 | N/A | The current PR diff changes pinned GPU test reference bundle metadata and CMake staging, not rocKE runtime example manifests under helpers/manifest.py. |
| 11 | N/A | The current PR diff does not modify runtime launch helpers, stream synchronization, WorkspacePool, or benchmark timing code. |
| 12 | N/A | The current PR diff does not touch COMGR or HIP library loading in runtime/comgr.py or runtime/hip_module.py. |
| 13 | N/A | The current PR diff only reorganizes test reference bundles by variant and does not refactor host-side grid, tile-count, workspace math, kernel-cache keys, or admission guards. |
| 14 | N/A | The current PR diff does not touch examples, expected.json files, or add new kernel/helper modules. |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The diffs for dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt (+7 -2 lines) and dnn-providers/hip-kernel-provider/rocke/platform/tests/test_gpu_references.py (+485 -80 lines) remain omitted to fit the model budget in the current diff, so CTest registration and installed test changes cannot be verified. |
| 2 | N/A | This PR refactors the GPU reference test bundle layout and does not add a new kernel family or variant to the platform IR or builders. |
| 3 | PASS | Tests and documentation grade the reference harness accurately, enforcing failure controls for perturbed outputs and missing launches in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:168-276 and noting subset verification caveats in dnn-providers/hip-kernel-provider/rocke/TESTING.md:154-156. |
| 4 | N/A | This PR does not re-bless IR goldens or widen the valid spec space in a lowerer or builder. |
| 5 | PASS | Tests remain portable across CPU/GPU and OS environments, skipping cleanly when hardware or bundles are absent (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:76-160) and querying the active device processor dynamically (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pytest_support.py:80-115). |
| 6 | PASS | The harness refactor preserves per-configuration semantics, scaling timeouts per installed variant (dnn-providers/hip-kernel-provider/rocke/TESTING.md:1009-1011), keeping at most one slot's workers alive at a time (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:82-163), and preserving error classes (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:227-232). |
| 7 | N/A | This PR does not modify engine backend rejection contracts or ROCKE_BACKEND / ROCKE_CPP_STRICT fallback behavior. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only restructures pinned GPU test reference harnesses and bundles and changes no kernel body, algorithm, tiling strategy, pipeline variant, or performance lever. |
| 2 | N/A | No performance lever or architecture-specific optimization recipe is modified in this test-reference refactoring PR. |
| 3 | N/A | No optimization lever covered by the utilities skills is touched by this test-infrastructure PR. |
| 4 | N/A | The diff does not alter kernel codegen, lowering, or performance primitives, so static ISA and occupancy probe evidence does not apply. |
| 5 | N/A | No global memory accesses in GPU kernels are added or modified by this diff. |
| 6 | N/A | No LDS or MMA layouts are added or changed in this pull request. |
| 7 | N/A | The PR makes no kernel bottleneck claims and implements no performance fixes. |
| 8 | N/A | No waits, barriers, or scheduling hints in GPU kernels are changed. |
| 9 | N/A | This PR is not a performance regression fix and changes no shape-dependent performance lever. |
| 10 | N/A | No compiler codegen changes, assembly parity claims, or performance benchmarks are included in this diff. |
| 11 | N/A | This PR introduces no new GPU optimization lessons, probes, or analysis tools. |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add a public rocke package symbol, <Op>Spec, build_<op>, grid helper, compile_kernel/KernelLauncher/time_launches parameter, run_manifest option, or rocke_* C entry point. |
| 2 | NEEDS-INFO | The previous NEEDS-INFO is resolved: existing pinned-reference cohort and contract tests are updated in the same PR to resolve variant adapters without dropping coverage at dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py:409 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:67. — no diff line supports this verdict |
| 3 | N/A | No public rocke package or C engine API is removed or renamed; the moved reference adapters are internal to library/tests/. |
| 4 | N/A | No __all__ list in rocke, rocke.core, rocke.runtime, rocke.analysis, or rocke.helpers.tiling is modified by this PR. |
| 5 | PASS | The test suites assert concrete contracts and negative failure cases, including wrong variant rejection at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:402 and cohort digest mismatch rejection at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:588. |
| 6 | PASS | CPU-checkable reference contracts and CLI commands are tested on host without a GPU at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312, while GPU execution and numeric checks are marked @pytest.mark.gpu at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:190. |
| 7 | N/A | The PR does not add or modify a platform API with a C++ twin or a C ABI binding. |
| 8 | NEEDS-INFO | The PR body lists reference suite runs, but does not state that PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=python:../library python tests/test_rocke.py and python tests/run_all.py from rocke/platform/ were run. |
A source-mode request without a variant, or with a null one, ran the processor's only variant, so the worker was the one reference tool that still relied on the single-variant default. Both workers now read the variant through reference_common.worker.source_variant, which rejects a missing or non-string variant before anything runs. qualify and verify_case already resolve the variant before they write a request, so no caller changes, and replay still reads only the processor's name. The physical-processor test now names a variant instead of relying on the default. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Thanks @yraparti. On the re-reviews at
|
|
Kicked off another multi-arch workflow for architectures not covered per PR: https://github.com/ROCm/rocm-libraries/actions/runs/41945289255 |
yraparti
left a comment
There was a problem hiding this comment.
rocKE checklist re-review of the fixes since e49fb56d9f82
ROCm/rocm-libraries#14164 · af517e79a8bd · model gemini-4-argon, fallback gpt-5.5
PASS · graded 238/238 · BLOCKER 0, HIGH 1, MEDIUM 0, LOW 0, NEEDS-INFO 63
The diff was truncated before grading. Items that needed a missing hunk are NEEDS-INFO.
Since the last review (e49fb56d9f82)
Resolved 1 · still open 1 · new 0
- ✅ §B.4 was FAIL, now PASS — had asked: In
dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:46anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:65, replacerequest.get("variant")withrequest["variant"]and requireisinstance(request["variant"], str)in… - ⏳ §D.1 still FAIL (HIGH): Put the knob on the KernelSpec and delete the environment read.
Findings
- HIGH §D ·
dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892· Put the knob on the KernelSpec and delete the environment read.
Needs info
- §C.1 Re-review resolves the prior NEEDS-INFO: repeated architecture/variant lookup is centralized in
AdapterRegistryatdnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and operation packages delegate to it atdnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10anddnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict - §C.2 New kernel-family code is exposed through spec-driven family builders rather than shape scripts, e.g.
DwFwdWmmaSpecis the family spec indnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:79and the builder takes that spec indnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:260. — no diff line supports this verdict - §C.5 The current re-review diff has inflated blast radius beyond the pinned-reference variant migration, adding unrelated benchmark fill tooling at
dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/conv_fill.py:1and new kernel implementation code atdnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:1. — no diff line supports this verdict - §C.8 New and revised helpers reuse shared current utilities instead of local reimplementations: dgrad WMMA code imports shared
align_up/ceil_divatdnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_dgrad_patch_wmma.py:53and reuses shared WMMA gate symbols atdnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_dgrad_smallc_wmma.py:53. — no diff line supports this verdict - §D.20 The diff is truncated around the platform/cpp instance hunks needed to verify C++ mirrors for the kernel-level Python selector/spec-builder changes; provide the untruncated C++ mirror hunks or parity-test lines for those changes.
- §E.13 The rebase onto
developsince the previous review did not resolve this:cli.py(truncated at line 1445),test_reference_common.py(truncated at line 1676),test_gpu_references.py, andreference_common/docs/*.mdare still truncated or omitted in the prompt, althoughdnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61anddnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107enforce that no pre-variant locks or DVC pointers remain. - §E.15 The rebase onto
developsince the previous review did not resolve this: all visible newvariantparameters inartifact.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read atdnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143),registry.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65), and the visible portion ofcli.py(dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349) are read and used, but the tail ofcli.py(after line 1445) andtest_gpu_references.pyare truncated/omitted. - §F.6 The new commits since e49fb56 do not include local
pre-commitexecution output, which cannot be verified from the diff alone.
Section notes
- §A: Re-review at head af517e7: the PR was rebased onto develop, which accounts for upstream commits in the e49fb56..af517e7 range; the PR's own diff against base (
Fileslist) remains strictly limited to pinned GPU reference test harnesses, documentation, and CMake reference staging. - §C: Current head includes additional rocKE kernel/benchmark/platform changes beyond the earlier reference-variant migration; item 5 is graded against that expanded blast radius.
- §D: This PR only reorganizes pinned GPU reference test bundles and harnesses by (operation, architecture, variant); it does not touch dispatcher registration, routing, or KernelSpec definitions.
- §D: The current diff is truncated; the C++ mirror hunks needed for item 20 are not fully visible.
- §G: Re-reviewed items 18-34 against head af517e7; the branch update since e49fb56 is a base-branch sync while the PR's own file set remains scoped to pinned GPU reference variant splitting.
- §H: Trace from visible hunks only; many kernel hunks are omitted/truncated, so this trace is incomplete. Visible conv_direct_dw_fwd_wmma: buffer SRD extents come from launch/runtime params A_bytes/B_bytes/D_bytes, while the visible compile-time visit space is grid=(ceil(Wo/tile_w), ceil(groups/block_ch), ceil(N/w_fold)ceil(Ho/block_h)); the spec only bounds sentinel-addressable X/Y bytes, so agreement with actual launch tensors cannot be fully checked from the truncated diff. Visible masks/bounds: ch_ok source is runtime p_groups vs compile-time channel chunk block_ch; s range is compile-time KW; output drain appears to combine runtime p_N/p_Wo/p_G with compile-time FTW*PER_PX but the hunk is truncated. Visible attention_dense_spec: seqlen_q/seqlen_kv are spec fields checked against block_m/block_n for ragged/sliding-window, not runtime kv_lens/cu_seqlens. No visible block_tables/paged indirection values are introduced, but omitted hunks prevent a complete trace.
Graded with the rocKE PR checklist, one model pass per section. FAIL rows that sit on a diff line are inline comments.
§A graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff against base (dnn-providers/hip-kernel-provider/rocke/library/tests/... and platform/cmake/InstallGpuReferences.cmake) touches only pinned GPU reference test infrastructure and no kernel-body emitters. |
| 2 | N/A | No core/lower_* or core/ir files are modified in this PR's file list. |
| 3 | N/A | This PR does not modify any kernel selectors, spec builders, lowering, or codegen knobs. |
| 4 | N/A | This PR modifies no Python or C++ IR emitters. |
| 5 | N/A | No representative-IR or library IR golden files are touched in this PR (only test reference baseline_lock.json and .dvc files are moved/updated). |
| 6 | N/A | This PR makes no IR or engine changes requiring parity or golden proof. |
| 7 | N/A | No kernel or lowering logic is added or modified on any IR emission path. |
| 8 | N/A | No C++ rocKE engine, lowering, helper, or instance code is touched in this PR. |
| 9 | N/A | No shared op-handler fix or lowerer arch divergence is introduced in this PR. |
| 10 | N/A | No new kernel or IR feature flag is introduced in this PR. |
| 11 | N/A | No new IR op is added in this PR. |
| 12 | N/A | core/backend.py and ROCKE_BACKEND handling are not touched by this PR. |
| 13 | N/A | .ll emission and IR gate environments are not modified by this PR. |
| 14 | N/A | No classic helper files under platform/python/rocke/helpers/ or platform/cpp/helpers/ are modified by this PR. |
§B graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff against base (develop) only modifies test reference infrastructure, CMake staging, and docs, and does not add any kernel or dispatcher rejection gates. |
| 2 | N/A | This PR does not touch any kernel spec constructors or preflight gates. |
| 3 | N/A | This PR only reorganizes pinned GPU reference test bundles by variant and does not change kernel routing or fallback behavior. |
| 4 | PASS | The previous FAIL is resolved: source_variant in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:55 now reads request["variant"] and requires isinstance(variant, str), and dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:52 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:71 call get_adapter(architecture, source_variant(request)) in… |
| 5 | PASS | The new variant-selection, cohort-digest, and CMake reference-wiring gates (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:77) are covered by host tests (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:402… |
| 6 | N/A | This PR does not add or modify any is_valid_spec or supports_* predicates. |
§C graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | Re-review resolves the prior NEEDS-INFO: repeated architecture/variant lookup is centralized in AdapterRegistry at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and operation packages delegate to it at dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/__init__.py:10 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/__init__.py:10. — no diff line supports this verdict |
| 2 | NEEDS-INFO | New kernel-family code is exposed through spec-driven family builders rather than shape scripts, e.g. DwFwdWmmaSpec is the family spec in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:79 and the builder takes that spec in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:260. — no diff line supports this verdict |
| 3 | PASS | Names are operation/algorithm/knob based: the new depthwise forward WMMA spec defaults to rocke_direct_dw_fwd_wmma in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:87, while reference variants use family names such as implicit_gemm_fwd at dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4 and dense at… |
| 4 | PASS | Arch-independent reference lookup lives in common AdapterRegistry at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, processor packages now only name the processor at dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/__init__.py:9, and new shared conv kernel logic is placed under library/kernels/common at… |
| 5 | NEEDS-INFO | The current re-review diff has inflated blast radius beyond the pinned-reference variant migration, adding unrelated benchmark fill tooling at dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/conv_fill.py:1 and new kernel implementation code at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:1. — no diff line supports this verdict |
| 6 | PASS | The reference migration is refactor-first through the shared AdapterRegistry abstraction at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, and the benchmark fill behavior is factored into a shared helper instead of copied per benchmark at dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/conv_fill.py:40. |
| 7 | PASS | The reference bundle migration preserves the DVC object identity while changing only the archive path, e.g. dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:2 and dnn-providers/hip-kernel-provider/rocke/library/tests/reference_bundles/conv/gfx942/implicit_gemm_fwd.tar.gz.dvc:5, and locks now pin the cohort digest at… |
| 8 | NEEDS-INFO | New and revised helpers reuse shared current utilities instead of local reimplementations: dgrad WMMA code imports shared align_up/ceil_div at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_dgrad_patch_wmma.py:53 and reuses shared WMMA gate symbols at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_dgrad_smallc_wmma.py:53. — no diff line supports this verdict |
| 9 | PASS | The variant source/replay bug class is handled across sibling operation workers: convolution dispatches source mode through the selected adapter at dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/worker.py:45, SDPA applies the same source/replay split at dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/worker.py:64, and CMake rejects stale pre-variant locks at… |
| 10 | PASS | The widened shared key (operation, architecture, variant) is propagated through option parsing at dnn-providers/hip-kernel-provider/rocke/library/tests/conftest.py:55, bundle path construction at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/paths.py:23, pytest parametrization at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:178, and installation staging at… |
§D graded 21/21
| # | Verdict | Evidence |
|---|---|---|
| 1 | FAIL | dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:892 reads the environment |
| 2 | N/A | This PR does not add or modify any dispatcher KernelCandidate or Capability declarations. |
| 3 | N/A | This PR does not add, modify, or call any dispatcher candidate _supports or admits predicates. |
| 4 | N/A | This PR does not touch dispatcher selectors or spec builders on the selection path. |
| 5 | N/A | This PR does not modify any KernelSpec identity, kernel_name, or cache-key logic. |
| 6 | N/A | This PR does not modify cohort routing, spec builders, launch metadata, or cache keys in the dispatcher. |
| 7 | N/A | This PR does not add or modify dispatcher heuristics, hand-pinned overrides, or resolvers. |
| 8 | N/A | This PR does not define or modify any dispatcher routing cohorts or spec override predicates. |
| 9 | N/A | This PR does not register any new kernel or architecture in the dispatcher registries. |
| 10 | N/A | This PR does not introduce any new KernelSpec fields or features. |
| 11 | N/A | This PR does not register, promote, or change precedence or opt-in status for any dispatcher candidates. |
| 12 | N/A | This PR does not add, rename, or reprioritize production dispatcher candidates in the registry priority table. |
| 13 | N/A | This PR does not modify CandidateRegistry.select or add a production dispatcher wrapper or entry point that could catch PinRefused and retry with auto. |
| 14 | N/A | This PR does not change dispatcher compiled-binary caches, routing logs, tuning records, KernelId identity, or candidate kernarg ABI versions. |
| 15 | N/A | This PR does not add or edit production dispatcher candidate registration files or arch modules as dispatcher candidates. |
| 16 | N/A | This PR does not introduce a new dispatcher operator family requiring registered combos, sweep space, or dispatch-all helpers. |
| 17 | N/A | This PR does not add a production dispatcher candidate whose CPU routing invariants or family fixture coverage must be extended. |
| 18 | N/A | This PR does not modify CandidateRegistry ratchets, binding requirements, route-only registries, or binding exempt sets. |
| 19 | N/A | This PR does not over-claim unified attention auto routing through run_unified_attention_torch or change that production auto path. |
| 20 | NEEDS-INFO | The diff is truncated around the platform/cpp instance hunks needed to verify C++ mirrors for the kernel-level Python selector/spec-builder changes; provide the untruncated C++ mirror hunks or parity-test lines for those changes. |
| 21 | N/A | This PR does not present a dispatcher chooser bug fix where a selected spec should be changed instead of editing emitted artifacts. |
§E graded 17/17
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | library/tests imports rocke (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:70), while platform/cmake/InstallGpuReferences.cmake invokes artifact.py via an isolated subprocess (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:208) without importing library into platform. |
| 2 | PASS | Device queries in the reference test infrastructure use from rocke.runtime.hip_module import get_device_target_id (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/worker.py:72) rather than torch. |
| 3 | PASS | Torch remains optional: dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/check_torch_reference.py:13 gates with pytest.importorskip("torch") and pinned reference tests verify assert not report["torch_imported"] (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:210). |
| 4 | N/A | This PR only modifies files under library/tests/, platform/, and TESTING.md, not production library/{kernels,dispatch,builders,benchmarks} layering or KNOWN_VIOLATIONS/KNOWN_BAD lists. |
| 5 | PASS | Platform CMake rules use relative directory variables (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:20) and Python helpers use tempfile and pathlib (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:18). |
| 6 | N/A | This PR does not touch emitter trees (core/, helpers/, instances/, library/kernels/). |
| 7 | PASS | Testing and workflow documentation are updated alongside the reference layout changes (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/docs/conv-test-reference.md:13, dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:386). |
| 8 | PASS | No Jira/tracking IDs, reviewer handles, or PR numbers appear in code or comments in the diff (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:1). |
| 9 | PASS | All PR-modified files are scoped to splitting pinned GPU reference bundles by operation, architecture, and variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | The PR was rebased onto current develop and explicitly guards against concurrent branches restoring pre-variant locks or DVC pointers at legacy paths (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61, dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107). |
| 11 | PASS | Only deliverable test harness code, locks, DVC pointers, CMake rules, and required reference documentation are committed (dnn-providers/hip-kernel-provider/rocke/TESTING.md:123, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22). |
| 12 | PASS | All enrolled architectures (gfx942, gfx950, gfx1151, gfx1200, gfx1201) are migrated to the variant layout (dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:483, dnn-providers/hip-kernel-provider/rocke/platform/cmake/PublishedGpuReferences.cmake:11). |
| 13 | NEEDS-INFO | The rebase onto develop since the previous review did not resolve this: cli.py (truncated at line 1445), test_reference_common.py (truncated at line 1676), test_gpu_references.py, and reference_common/docs/*.md are still truncated or omitted in the prompt, although dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:61 and dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:107 enforce that no pre-variant locks or DVC pointers remain. |
| 14 | PASS | After rebasing onto develop, moved adapter files preserve upstream comments, historical API compatibility checks, and docstrings (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/__init__.py:4, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/__init__.py:43). |
| 15 | NEEDS-INFO | The rebase onto develop since the previous review did not resolve this: all visible new variant parameters in artifact.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:109, read at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143), registry.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65), and the visible portion of cli.py (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:349) are read and used, but the tail of cli.py (after line 1445) and test_gpu_references.py are truncated/omitted. |
| 16 | N/A | This item is a meta-instruction for review grading rather than a property of the pull request diff. |
| 17 | PASS | No performance numbers or superiority claims appear in the PR description, docs, or code (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§F graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | PASS | Branch users/brockhargreaves-amd/pinned-reference-variants matches users/<user>/… (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
| 2 | PASS | title test(rocke): split pinned GPU references by operation, architecture, and variant matches type(scope): summary and is 80 characters |
| 3 | PASS | description is at least 30 characters and contains an issue reference |
| 4 | PASS | test file dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py is in the diff |
| 5 | PASS | no key, certificate, or secret filename in the diff |
| 6 | NEEDS-INFO | The new commits since e49fb56 do not include local pre-commit execution output, which cannot be verified from the diff alone. |
| 7 | NEEDS-INFO | The changes since e49fb56 do not include GitHub review thread metadata, so bot/Copilot thread resolution and deferred ticket IDs cannot be verified. |
| 8 | PASS | Base branch is develop (body notes Stacked on #13651), and the branch prefix, Conventional Commit title, description with issue ref, unit tests, and absence of forbidden files satisfy policy.yml (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312). |
| 9 | PASS | The full variant-keyed reference migration and test suite remain together in one PR without artificial splitting for a size cap (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:32). |
| 10 | PASS | Tests in dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:353 exercise real variant discovery, cohort digesting, and CMake staging invariants rather than throwaway checks. |
| 11 | PASS | The fetched PR metadata shows the populated Title and complete Motivation/Technical Details/Test Plan/Test Result Body are intact (dnn-providers/hip-kernel-provider/rocke/TESTING.md:120). |
§G graded 34/34
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | No visible change uses byte-identity or parity as a correctness proof rather than a parity/golden check. |
| 2 | NEEDS-INFO | New kernel work is present in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:4, but the diff provides no on-silicon fp32-reference numeric completion evidence for that kernel work. — no diff line supports this verdict |
| 3 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: current changes add kernel/platform emission work, but the diff does not include output proving tools/run_checks.py from the rocKE root, the llvm23 byte-identity check, installed-golden IR, IR-validity, and relevant pytest/numeric gates were run or shown unchanged versus develop. |
| 4 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: many added behavior/test hunks are omitted from the diff, including direct-kernel tests, platform tests, and the tail of reference_common/cli.py, so fail-before coverage cannot be verified for all new behavior. |
| 5 | NEEDS-INFO | Previous NEEDS-INFO is not resolved: visible reference tests exercise shipped paths, but omitted test hunks for new kernels, platform changes, and CLI/status paths prevent verifying falsifiability and reachability for all added tests. |
| 6 | NEEDS-INFO | New problem in current commits: the comment in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:66 says the block-wave cap is lower than the actual cap set at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:69, so load-bearing control/limit prose contradicts the code. — no diff line supports this verdict |
| 7 | N/A | This diff does not introduce a new design-doc policy for an fp32 parity-reference oracle. |
| 8 | N/A | No visible rocKE harness change adds or modifies an fp8-to-fp16 silent dtype fallback. |
| 9 | NEEDS-INFO | The new kernel adds load-bearing fragment/VGPR budget prose and constants at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:66 without a tool-sourced provenance note in the visible diff. — no diff line supports this verdict |
| 10 | N/A | The visible rocKE diff changes benchmark plumbing but makes no specific kernel performance claim requiring device, ratio, shipped-script, and parity provenance. |
| 11 | NEEDS-INFO | The current diff adds optimized kernel builders, but selector/caller and measurement hunks are omitted, so reachability through Python routing and live C++ selectors on a real measured path cannot be verified. |
| 12 | NEEDS-INFO | The current diff includes cross-engine Python/C++ kernel changes, but many C++ instance, binding, selector, and caller hunks are omitted, so the who-calls-it-now cross-layer gap cannot be graded. |
| 13 | N/A | No visible rocKE change recommends a mangled intrinsic symbol requiring .td overload-class verification. |
| 14 | N/A | The visible rocKE diff does not classify a specific performance bottleneck that would require dynamic hardware-counter evidence. |
| 15 | N/A | No wrong-branch risk is apparent because the review is against the provided current-head diff. |
| 16 | NEEDS-INFO | Previous NEEDS-INFO is resolved: AdapterRegistry is defined in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:22 and both operation packages delegate to it in dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/init.py:12 and dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/init.py:12. — no diff line supports this verdict |
| 17 | NEEDS-INFO | New shipped scripts and script-like benchmark/CLI paths were added or modified in the current head, but several relevant test hunks are omitted, so end-to-end execution coverage beyond import/parse cannot be verified for all shipped scripts. |
| 18 | PASS | Variant and cohort-digest guards are wired into bundle validation and CLI manifest authentication (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:143, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/artifact.py:163, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:604), and unit tests construct mismatched variant and cohort-digest cases… |
| 19 | PASS | Test fixtures construct multi-variant, sole-variant, published, unpublished, missing-bundle, and candidate-bundle states and assert each gate's inline handling (dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1273, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1468, dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:1596). |
| 20 | N/A | The PR's file list touches only pinned GPU reference test organization and documentation and does not add or modify a claim-ban detector. |
| 21 | PASS | In the reference qualification and verification harness touched by this PR, case error budgets, tolerances, and cohort digests are explicitly validated and propagated (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:298, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:815), and the forward conv benchmark fill helper synced in the branch update documents its floor semantics… |
| 22 | PASS | Explicitly selected pinned-reference runs fail rather than skip when a published bundle is missing or when an unpublished variant is selected without a candidate bundle and lock (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:85, dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:145), and CMake fails configuration when published locks or archives are missing… |
| 23 | N/A | This PR reorganizes pinned GPU reference test bundles by variant and makes no performance or kernel-routing speedup claims. |
| 24 | PASS | The pinned GPU reference locks are migrated to variant subdirectories with cohort_sha256 added while preserving the existing baseline_revision and manifest_sha256 digests unchanged (dnn-providers/hip-kernel-provider/rocke/library/tests/conv_reference/architectures/gfx942/implicit_gemm_fwd/baseline_lock.json:1, dnn-providers/hip-kernel-provider/rocke/library/tests/sdpa_reference/architectures/gfx942/dense/baseline_lock.json:1). |
| 25 | N/A | This PR does not ship a codegen micro-optimization. |
| 26 | N/A | This PR does not report kernel performance benchmarks. |
| 27 | N/A | This PR does not perform or report a performance A/B experiment. |
| 28 | N/A | This PR does not report performance deltas or counterfactual benchmark tables. |
| 29 | N/A | This PR does not swap guarded math primitives for fast approximations. |
| 30 | N/A | This PR makes no register-spill or scratch-memory claims. |
| 31 | N/A | This PR does not claim or benchmark a fix for a kernel performance defect. |
| 32 | N/A | This PR reports test pass/skip counts rather than measured performance or hardware resource figures. |
| 33 | PASS | The new variant resolution and selection logic in AdapterRegistry.resolve_variant (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/registry.py:65) and select_bundle (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:85) is exercised across unenrolled, ambiguous, explicit-override, sole-variant, published, and unpublished combinations… |
| 34 | N/A | This PR does not add or modify an opt-in kernel output flag. |
§H graded 5/5
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The visible new kernel creates buffer resources from runtime A_bytes/B_bytes/D_bytes, but multiple kernel hunks that may create SRDs or launch validation are omitted/truncated; provide the full diff for all buffer_rsrc/resource-bound construction plus the host-side launch validation and negative tests. |
| 2 | NEEDS-INFO | No block-table or paged-indirection value lookup is visible in the supplied hunks, but the kernel/attention hunks are omitted/truncated, so it is not possible to confirm that no loaded value is used as a memory index or that such values are range-checked. |
| 3 | NEEDS-INFO | The visible kernel uses div/mod by compile-time constants, but the allowed constant sets and additional div/mod sites are in omitted/truncated hunks; provide the full spec validation showing those divisors are constrained to powers of two or the benchmark/justification for arbitrary divisors. |
| 4 | N/A | The visible diff does not introduce runtime sequence-length inputs such as kv_lens or cu_seqlens that build a device-side softmax mask over a compile-time attention tile range. |
| 5 | NEEDS-INFO | Visible runtime launch inputs feed device-side bounds/masks, including tensor byte bounds and geometry values, but the host-validation and negative-test hunks for all such inputs are omitted/truncated, so symmetry across sibling inputs cannot be checked. |
§I graded 6/6
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The benchmark path measures the candidate space rather than only a preselected winner: _measure_plans is documented to time every plan of one case, and cache mode enumerates compatible candidates before optional narrowing by a kernels file (dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/benchmark_direct_conv.py:2234-2250; dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/benchmark_implicit_gemm_conv.py:4740-4778). — no diff line supports this verdict |
| 2 | PASS | Invalid or half-wired reference states are rejected at configure time with explicit causes, including unenrolled published architectures, pre-variant locks, invalid variant names, missing adapters, missing publication entries, and missing DVC pointers (dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:47-87; dnn-providers/hip-kernel-provider/rocke/platform/cmake/InstallGpuReferences.cmake:94-118). |
| 3 | N/A | This change reorganizes pinned reference variants and related tooling; it does not alter a curated/generated coverage policy or add Hypothesis-style generated coverage. |
| 4 | NEEDS-INFO | The relevant benchmark result-emission/schema hunks are omitted or truncated, so it is not possible to verify whether shipped and achievable columns are both retained. |
| 5 | PASS | Qualified bundles are explicit snapshots: qualification records the selected variant in the manifest and writes a lock with baseline_revision, cohort_sha256, and manifest_sha256, while artifact.py published provides a regeneration/tooling path for locating published variant archives and locks (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:470-489… |
| 6 | NEEDS-INFO | The re-review changes mention platform primitive/lowering and hardware-pattern instance files, but their hunks are omitted; the full diffs for the public API/intrinsics, lowering, and equivalence tests are needed to verify this ★ interface rule. |
§J graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The modified Markdown visible in the diff (TESTING.md, conv-test-reference.md, platform/tests/README.md) contains no mathematical equations or matrix products to type-check. |
| 2 | N/A | The PR does not introduce mathematical formula notation tables or multi-domain quantities such as decay versus log-decay. |
| 3 | N/A | The visible Markdown documents GPU reference packaging and test workflows rather than restating a kernel algorithm derivation owned by another design document. |
| 4 | NEEDS-INFO | Not resolved from the previous NEEDS-INFO: the rebase commit (e49fb56..af517e7) did not make the omitted PR Markdown hunks visible (dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/docs/README.md, gpu-ci-pinned-rocke-test-reference-plan.md, gpu-reference-workflow.md, quickstart.md, and sdpa_reference/docs/sdpa-test-reference.md remain omitted to fit the model budget), so novelty claims in those files still cannot be verified. |
| 5 | PASS | dnn-providers/hip-kernel-provider/rocke/TESTING.md:1010-1011 and dnn-providers/hip-kernel-provider/rocke/platform/tests/README.md:389-390 state the CTest timeout as a per-variant aggregate (five minutes per installed variant / five minutes per variant). |
| 6 | N/A | This PR reorganizes pinned test reference bundles and does not add model config.json tables, tuned dispatch band edges, or kernel constants requiring deployment provenance. |
| 7 | N/A | No new Markdown design or algorithm document file is added in the PR file list; existing test documentation files are updated in place. |
§K graded 13/13
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR only reorganizes pinned GPU reference test bundles and adapters under library/tests/ and does not add or modify any tuned dispatch candidates or KnobSpace subclasses. |
| 2 | N/A | This PR does not add or modify any tunable kernel spec fields or KnobAxis declarations. |
| 3 | N/A | This PR does not change tuning_id, config_key, knob canonicalization rules, or tuned spec defaults. |
| 4 | N/A | This PR does not touch KnobSpace.defaults() or KnobSpace.recorded(). |
| 5 | N/A | This PR does not modify knob dropping, refusal, inertness, or base_value() rules. |
| 6 | N/A | This PR does not modify KnobSpace.validate or policy-free tuning spec construction. |
| 7 | N/A | This PR does not touch KnobSpace.base(req) or request normalization for tuning. |
| 8 | N/A | This PR does not change normalize_knobs or knob value coercion. |
| 9 | N/A | This PR modifies GPU test reference bundle replay under library/tests/, not dispatch tuning_id/spec_hash benchmark replay. |
| 10 | N/A | This PR does not touch KnobSpace.find or tuning_id resolution in dispatch. |
| 11 | N/A | This PR does not touch dispatch sweep entry points, pin_to_spec, or sweep harnesses. |
| 12 | N/A | This PR does not change production stacks, accept_default, or base specs for auto tuning selection. |
| 13 | N/A | This PR does not touch dispatch tuning sweep harnesses or make claims about auto selection. |
§L graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This diff does not show a new gfx hardware target being added to the rocKE architecture fact sources or ISA backend registries. |
| 2 | NEEDS-INFO | The diff is truncated for relevant MMA lowering changes such as dnn-providers/hip-kernel-provider/rocke/platform/cpp/core/lower_hip/lower_hip_mma.cpp and does not include enough hunks to determine whether any new MMA op_id was added with matching fragment/layout metadata. |
| 3 | NEEDS-INFO | The diff is truncated for relevant WMMA/MFMA helper and kernel changes such as dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_fwd_wmma_gate.py and multiple new WMMA kernels, so I cannot verify that all MMA selection and lowering goes through the catalog and does not hard-code intrinsic names or lane maps. |
| 4 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:81 hard-codes DW_FWD_WMMA_WAVE = 32 instead of deriving the wave size from the exact ArchTarget.wave_size. — no diff line supports this verdict |
| 5 | NEEDS-INFO | The diff is truncated for several arch-specific recipe files and new WMMA kernels, so I cannot verify that gfx942/gfx950/gfx1250 recipes and loaders remain properly gated and do not leak across targets. |
| 6 | NEEDS-INFO | The diff is truncated for relevant policy/instance and platform Python changes such as dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/instances/gfx1250/block_scaled_gemm.py and related files, so I cannot verify pipeline vocabulary placement or import boundaries. |
| 7 | NEEDS-INFO | The diff is truncated for relevant dtype and packing changes including NVFP4/scale-format examples, tests, and lowering files, so I cannot verify the logical dtype, bit-packing, scalar-lowering, and FP8 dialect contracts. |
| 8 | NEEDS-INFO | The diff is truncated for relevant scaled-MMA changes in lowering, examples, and golden files, so I cannot verify all-or-nothing scale contracts or that retired scaled WMMA ids were not reintroduced. |
| 9 | N/A | This diff does not introduce a new architecture target, so the new-arch completion gates do not apply. |
| 10 | NEEDS-INFO | The diff is truncated for dnn-providers/hip-kernel-provider/rocke/platform/tests/tools/test_arch_domain_artifact.py and related tooling changes, so I cannot verify that arch-domain artifacts and LLVM flavor membership remain generated and non-hand-rolled. |
§M graded 10/10
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff only modifies test reference harnesses, locks, and CMake scripts under rocke/library/tests/ and rocke/platform/, with no changes to core/ IR, builders, or kernel control flow. |
| 2 | N/A | The PR diff does not touch legalize_for_target, architecture data, or lowering files. |
| 3 | N/A | The PR diff does not touch ck.dsl.ir serialization, print_ir, wire versions, attribute tagging, SSA ids, or ABI parameter ordering. |
| 4 | N/A | The PR diff does not touch portable IR schemas, importers, recipe expansion/VM, or recording builders. |
| 5 | N/A | The PR diff does not touch AsyncTileLoader, LDS layouts, or kernel memory lowering. |
| 6 | N/A | The PR diff does not emit or modify kernel control flow, barriers, memory loads, or cross-lane shuffles. |
| 7 | N/A | The PR diff does not touch ISA backends, buffer resource word 3, or TemporalHint handling. |
| 8 | N/A | The PR diff does not add intrinsic declarations, modify LLVM profile tables, or change backend selection precedence. |
| 9 | N/A | The PR diff does not touch public C entry points, headers under cpp/include/rocke/, or C++ host launch behavior. |
| 10 | N/A | The PR diff does not emit or consume LLVM debug info (!dbg) or DWARF metadata. |
§N graded 18/18
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR diff only touches GPU reference test harness files and CMake rules, without adding or modifying helpers under platform/python/rocke/helpers/. |
| 2 | N/A | The PR diff does not author or modify kernels or their use of shared helper mechanisms. |
| 3 | N/A | The PR diff does not touch SoftwarePipeline or any kernel pipeline code. |
| 4 | N/A | The PR diff does not add or modify fused kernels, epilogue fusion logic, fusion validation, or fusion legalizers. |
| 5 | N/A | The PR diff does not introduce or change any public tiling API under helpers/tiling/. |
| 6 | N/A | The PR diff does not add or modify front-door or toolbox tiling helpers or any LayoutStyle implementation. |
| 7 | N/A | The PR diff does not touch tiling layout overrides, accumulator descriptors, or MMA validation. |
| 8 | N/A | The PR diff does not touch tiling memory operations, LDS allocation, global load/store emission, clipping, padding, or wide-load selection. |
| 9 | N/A | The PR diff does not use or modify N-D tensor axis roles, batch reductions, flow-stage relabeling, or AB-swap routing in tiling. |
| 10 | N/A | The PR diff does not modify tiling emit, LDS staging, MMA issue logic, or tiling fail-fast raises. |
| 11 | N/A | The PR diff does not touch MMA traits, TileMma, lds_swizzle, scheduling group helpers, or related tiling policies. |
| 12 | N/A | The PR diff makes no LDS bank-conflict or coalescing claims and does not modify related simulator or coalescing analysis tools. |
| 13 | N/A | The PR diff does not construct or modify WarpDistributionEncoding, make_tile_desc, RegisterMapper, or tiling store casts. |
| 14 | N/A | The PR diff does not use tiling verbs, modify synchronization or barrier placement, or add a tiling pipeline variant. |
| 15 | N/A | The PR diff does not touch tiling allocation or bounds-checking logic for transformed address ranges. |
| 16 | N/A | The PR diff does not use or modify optimize_layout, recommend, or evaluate_transform. |
| 17 | N/A | The PR diff does not touch the tiling recorder, round-trip analysis, witness flags, or visualization modules. |
| 18 | N/A | The PR diff does not touch interleaved layouts, tiling demo kernels, dispatch registration of tiling demos, or tiling numeric gates. |
§O graded 16/16
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | This PR does not touch core/verify.py, cpp/core/verify.cpp, or IR address spaces/attributes. |
| 2 | N/A | This PR does not modify scf.for, region structure, or IR block emission. |
| 3 | N/A | This PR does not modify IRBuilder, LLVM intrinsic declaration tables, or metadata emission. |
| 4 | N/A | This PR does not modify IR types, SmemType, or IR parsers. |
| 5 | N/A | This PR does not modify core/passes.py, cpp/core/passes.cpp, or op purity tables. |
| 6 | N/A | This PR does not touch constant folding in Python or C++ passes. |
| 7 | N/A | This PR does not modify kernel attributes, COMGR options, or cluster dimension checks. |
| 8 | N/A | This PR does not touch CodegenPolicy, SchedulerStrategy, or kernel compilation options. |
| 9 | N/A | This PR does not modify waitcnt encoding or barrier lowering. |
| 10 | N/A | This PR does not touch LLVM lowerers or parameter attribute/type lowering. |
| 11 | N/A | This PR does not touch ArchTarget or target catalog resolution. |
| 12 | N/A | This PR does not touch vector/LDS width caps or codegen arch defaults. |
| 13 | N/A | This PR does not touch public C headers under cpp/include/rocke/ or ABI versions. |
| 14 | N/A | This PR does not touch C++ engine sources, headers, bindings, or C++ compile flags in platform/CMakeLists.txt. |
| 15 | N/A | This PR does not touch C++ engine fallback handling or alternative lowerers. |
| 16 | N/A | This PR does not touch known_gaps.md or C++ library/builder capabilities. |
§P graded 14/14
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:260 adds build_dw_fwd_wmma(spec, arch="gfx1250"), so arch is a positional/defaulted parameter rather than keyword-only (spec, *, arch). — no diff line supports this verdict |
| 2 | NEEDS-INFO | The changes-since list shows ABI-relevant files such as platform/cpp/include/rocke/abi.h and platform/python/rocke/portable_ir/src/abi.py, but their hunks are omitted, so kernarg order and packing compatibility cannot be verified. |
| 3 | NEEDS-INFO | The changes-since list includes platform instance changes such as platform/python/rocke/instances/gfx1250/block_scaled_gemm.py, but the relevant launch-geometry hunks are omitted, so grid-source compliance cannot be verified. |
| 4 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:234 gates spec.lds_bytes() against the literal DW_DGRAD_MFMA_LDS_BUDGET instead of the architecture target LDS capacity/fits helper. — no diff line supports this verdict |
| 5 | NEEDS-INFO | The changes-since list includes gfx1250 GEMM instance files and WMMA-related platform changes, but their hunks are omitted, so narrow WMMA/RDNA GEMM admission rules and wave-size validation cannot be checked. |
| 6 | N/A | The shown rocKE diff does not touch Stream-K, MoE sorting, fused MoE, or split-K workspace sizing or zeroing. |
| 7 | NEEDS-INFO | The changes-since list includes platform/python/rocke/instances/SUPPORT_MATRIX.md, but its hunk is omitted, so preserved rejection limits for Stream-K, persistent GEMM, grouped GEMM, and the support matrix cannot be verified. |
| 8 | NEEDS-INFO | The changes-since list includes quantization/NVFP4 and MX-related instance, example, and test files, but the relevant hunks are omitted, so use of the shared quantization helpers and MX constraints cannot be verified. |
| 9 | NEEDS-INFO | The changes-since list includes numeric emulation and NVFP4 test files, but the relevant hunks are omitted, so edge-case decode/emulation coverage in both engines cannot be verified. |
| 10 | N/A | The shown diff changes pinned GPU reference bundle metadata and CMake staging, not rocKE runtime example manifests under helpers/manifest.py. |
| 11 | N/A | The shown diff does not modify rocKE runtime launch helpers, stream synchronization, WorkspacePool, or benchmark timing infrastructure covered by this item. |
| 12 | N/A | The shown diff does not touch COMGR or HIP library loading code in rocKE runtime. |
| 13 | NEEDS-INFO | The changes-since diff visibly refactors host grid/tile math with shared ceil_div/align_up helpers, but the relevant numeric-test hunks and per-consumer proof are omitted, so the required consumer test coverage cannot be verified. |
| 14 | NEEDS-INFO | dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:4 adds new kernel-family logic as another flat library/kernels/common module instead of placing new kernel logic under the appropriate instances/family structure required by this item. — no diff line supports this verdict |
§Q graded 7/7
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | The previous NEEDS-INFO is not resolved in the visible current diff: the relevant registration hunks for dnn-providers/hip-kernel-provider/rocke/platform/CMakeLists.txt and dnn-providers/hip-kernel-provider/ROCKE_ENGINE_test_categories_external.yaml are still omitted, so CI/CTest registration and lane selection cannot be verified. |
| 2 | NEEDS-INFO | Current head adds new kernel variants, but the hunk for dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/rocke_ir_parity_harness.py is omitted, so I cannot verify that cases() covers one case per variant per arch for byte-identity lowering. |
| 3 | NEEDS-INFO | Current head changes platform gates and related tests, but relevant hunks such as platform/tools/check_byte_identity.py, platform/tests/tools/test_run_checks.py, and platform/tests/instances/rocke_ir_parity_harness.py are omitted, so I cannot verify that statuses and gate claims are graded only for what each gate proves. |
| 4 | NEEDS-INFO | The current diff adds representative/differential golden files and lowerer/builder changes, but the diff does not include evidence that goldens were re-recorded from a green tree for every LLVM flavor or that fuzz_diff.py was run, or a stated reason why not. |
| 5 | NEEDS-INFO | The current head adds multiple platform tests with omitted hunks, including GPU/LLVM-specific tests and platform/tests/test_gpu_references.py, so portability details such as dynamic target/flavor selection, clean skips without HIP/device/clang, and newline normalization cannot be verified from the visible diff. |
| 6 | PASS | The refactor keeps per-variant installed timeout semantics in dnn-providers/hip-kernel-provider/rocke/TESTING.md:1007-1011, reuses at most one selected slot's workers at a time in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:76-163, and preserves worker failure error classes in dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/cli.py:226-232. |
| 7 | NEEDS-INFO | Current head changes backend/rejection-related code and tests, but the relevant hunks for files such as platform/python/rocke/core/backend.py and backend parity/rejection tests are omitted, so I cannot verify the ROCKE_BACKEND × ROCKE_CPP_STRICT × binding-present matrix or exact exception assertions. |
§R graded 11/11
| # | Verdict | Evidence |
|---|---|---|
| 1 | NEEDS-INFO | A redesign/new kernel body is visible in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:1, with tiling/pipeline knobs such as block_waves, block_h, w_fold, and prefetch_rows at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:114 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:117; missing the runbook Step 0 lever sweep, correctness pruning against fp32, and named best existing baseline. |
| 2 | NEEDS-INFO | The new kernel is arch-gated to gfx1250 at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:88 and validated through is_valid_dw_fwd_wmma_spec at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:229, but the diff does not provide the runbook §6/§7/§8 mapping, §13 convolution checklist evidence, arch/gfx1250.md recipe citation, or anti-pattern/failure-mode justification for the changed levers. |
| 3 | NEEDS-INFO | The visible levers include LDS sizing at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:162, launch geometry at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:138, and prefetch_rows at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:117; missing evidence that the PR followed the matching GEMM/LDS/prefetch/ISA/launch utilities skills or justified any contradiction to them. |
| 4 | NEEDS-INFO | The intended primitive changes to WMMA via wmma_fwd_op in dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:345 and the review-response diff also changes lowering/codegen files, but the PR evidence shown does not include before/after probe_occupancy.py, probe_regs.py, probe_intrinsic_counts.py or probe_isa_inspect.py, nor probe_lowering_compare.py for backend-affecting lowering changes. |
| 5 | NEEDS-INFO | The new kernel imports and uses buffer load/store helpers for global accesses at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:62 and dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:64, but the diff does not show analyze_coalescing/vector_transactions/addr_fn_from_strides summaries or assert_asm_backed gating against emitted ISA width for the added or changed accesses. |
| 6 | NEEDS-INFO | The new kernel allocates LDS staging at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:384 and uses WMMA at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:345, but the provided diff does not show verify_lds_roundtrip/verify_mma_soundness evidence, optimize_layout recommendations, or an equivalent bit-exact layout proof for this non-tiling kernel. |
| 7 | N/A | The visible rocKE diff does not make a bottleneck claim such as memory-bound, LDS-bound, barrier-bound, or latency-bound that would require runbook §3 or §8.6 classification evidence. |
| 8 | NEEDS-INFO | Scheduling validation changes are visible, for example replacing normalize_scheduler_strategy with scheduler_strategy_reason at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_dgrad_patch_wmma.py:245, and the new kernel describes row-level LDS/barrier staging at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:35; missing the runbook §8.3/§8.4 and scheduler-policy.md rationale plus ISA evidence for any changed waits, barriers, or scheduling hints. |
| 9 | NEEDS-INFO | The new gfx1250-only variant gate is at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:88 and its shape-dependent knobs are at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:114 through dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:117; missing the family shape sweep and any dispatch/heuristic gate for shapes that lose in the tradeoff. |
| 10 | NEEDS-INFO | The diff changes benchmark methodology through the new fill-mode helper at dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/conv_fill.py:1 and dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/conv_fill.py:16, and the review-response diff lists codegen/lowering changes, but the provided evidence does not show runbook §10 compiler-hazard/alias-semantics checks or §2.3 benchmark-hygiene details for any performance conclusions. |
| 11 | NEEDS-INFO | New optimization content is introduced by the Toeplitz WMMA depthwise forward kernel at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:1 and its new knobs at dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_direct_dw_fwd_wmma.py:114, but the diff does not add a corresponding optimization lesson to runbook §14/§16/§17 or the relevant utilities/skills documentation. — no diff line supports this verdict |
§S graded 8/8
| # | Verdict | Evidence |
|---|---|---|
| 1 | N/A | The PR does not add a public rocke package symbol, <Op>Spec, build_<op>, grid helper, compile_kernel/KernelLauncher/time_launches parameter, run_manifest option, or rocke_* C entry point. |
| 2 | NEEDS-INFO | The previous NEEDS-INFO is resolved: existing pinned-reference cohort and contract tests are updated in the same PR to resolve variant adapters without dropping coverage at dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_reference_contract.py:435 and dnn-providers/hip-kernel-provider/rocke/library/tests/test_sdpa_reference_contract.py:67. — no diff line supports this verdict |
| 3 | N/A | No public rocke package or C engine API is removed or renamed; the moved reference adapters are internal to library/tests/. |
| 4 | N/A | No __all__ list in rocke, rocke.core, rocke.runtime, rocke.analysis, or rocke.helpers.tiling is modified by this PR. |
| 5 | PASS | The test suites assert concrete contracts and negative failure cases, including wrong variant rejection at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:402 and cohort digest mismatch rejection at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:588. |
| 6 | PASS | CPU-checkable reference contracts and CLI commands are tested on host without a GPU at dnn-providers/hip-kernel-provider/rocke/library/tests/test_reference_common.py:312, while GPU execution and numeric checks are marked @pytest.mark.gpu at dnn-providers/hip-kernel-provider/rocke/library/tests/reference_common/pinned_tests.py:190. |
| 7 | N/A | The PR does not add or modify a platform API with a C++ twin or a C ABI binding. |
| 8 | NEEDS-INFO | The PR body lists reference suite runs, but still does not state that PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=python:../library python tests/test_rocke.py and python tests/run_all.py from rocke/platform/ were run. |
|
Automated review: Model - qwen3.6:35b-a3b-coding-mtp-q4_K_M, effort: max PR #14164 — split pinned GPU references by operation, architecture, and variant (brockhargreaves-amd, AICK-2299) — 🟠 Follow-up: fix verified; two quality should-fixes
Utility: 🟢 Positive — source requests now name their variant; pinned references pass on three architectures. 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: merge with develop, build + ctest, byte identity, goldens, the PR's tests (source and installed), a revert test of the worker fix, trial merges with open PRs, CI shard logs, code quality with a strict-registry probe and a variant-cap probe. Flags whether a closer look is needed; not an approval or rejection. |

Motivation
Pinned GPU reference bundles are currently keyed by (operation, architecture): each pair has one baseline lock, one DVC pointer, and one archive. As more kernel authors add pinned coverage, this creates two problems:
This PR keys references by (operation, architecture, variant), where a variant is a kernel implementation (for example convolution
implicit_gemm_fwd, SDPAdenseandwmma). Each variant owns its cases, baseline revision, lock, and archive, so independent kernel work lands in disjoint files and can pin its own baseline.Stacked on #13651.
Technical Details
Layout (paths under
rocke/library/tests/):<op>_reference/architectures/<arch>/<variant>/{__init__.py,baseline_lock.json}.<arch>/__init__.pyonly names the processor.reference_bundles/<op>/<arch>/<variant>.tar.gz(.dvc), with one root.gitignorefor archives and extracted bundles.engines/test_arch_content/rocke/<op>/<arch>/<variant>/. The architecture directory stays the first processor-named level, so per-target packaging is unchanged.implicit_gemm_fwd; SDPA gfx942/gfx950dense; SDPA gfx1151/gfx1200/gfx1201wmma(gfx1200 enrolled, unpublished).Migration without requalification:
.dvcpointer changes only itspath:line, with the same md5 and size, so no new DVC objects are needed.variantfield are identified by their lock's path.Commits:
cohort_sha256digest of each qualified cohort in its baseline lock. Configuration checks it against the manifest, and a host test checks it against the adapter'sCASES, so a cohort edit without requalification fails in every tree, even without bundle data.reference_common.--variantforqualify/verify/cases/diffand--rocke-reference-variantfor pytest, defaulting when an architecture has one variant. Test ids become<arch>-<variant>-<case>.variantin newly qualified manifests and validate it when present.status(one line per enrolled variant: publication, baseline, bundle state, shared cohorts) andartifact.py published [--json]for tooling.Unchanged:
set(ROCKE_PUBLISHED_<OP>_ARCHITECTURES ...)publication lists.architecturekey and the archive member root.Where new kernels go:
Case, independent reference, input generator, and manifest operation) is a new variant.This keeps each contract single-purpose and independent kernel PRs in disjoint files. The docs include a table of what can still overlap between concurrent PRs: the same variant, the shared WMMA cohort, adding an architecture, and contract changes.
Developer and tooling notes:
<op>/<arch>.tar.gzarchives and extracted bundles in existing checkouts are ignored leftovers: delete them anddvc pull./<variant>, or useartifact.py published --json.Test Plan
test_reference_common.py, conv/SDPA contract tests,test_sdpa_torch_reference.py,platform/tests/test_gpu_references.pywith its CMake configure/install checks) on every commit in the stack. Also run at the head with a temporary path containing a space and$.statuscohort test runs in both source and installed trees.--rocke-reference-operation/--rocke-reference-archon gfx942 and gfx950 (conv and SDPA), gfx1201 (SDPA), and gfx1151 (SDPA). Each run includes the three failure controls: perturbed current output, perturbed baseline replay, and a missing launch.status,cases, andverify --caseon gfx942.Test Result
$temporary path). Every commit passes.rocke_reference_common_pytest447 passed, 6 skipped. The 6 are the locked-baseline requalification checks, which need git history and skip in installed trees. The matchingrocke_<op>_gpu_<arch>_pytestpasses on gfx942, gfx950, and gfx1151; the other processors' entries skip; category selection is unchanged.statusreports both convolution variants matching their bundles with a shared cohort, andverify --casepasses one selected case with the subset caveat.Submission Checklist
JIRA ID : AICK-2299