Skip to content

[MIOpen] Register TransposedConvWinoRageRxS in the Find solver lists (ALMIOPEN-2606) - #11949

Merged
DarylHawkinsAMD merged 7 commits into
developfrom
users/dahawkin/register-transposed-wino-rage
Sep 14, 2026
Merged

DarylHawkinsAMD merged 7 commits into
developfrom
users/dahawkin/register-transposed-wino-rage

Conversation

@DarylHawkinsAMD

@DarylHawkinsAMD DarylHawkinsAMD commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Summary

TransposedConvWinoRageRxS<2,3> was registered in the MIOpen solver registry (id 191) and
explicitly instantiated, but was missing from both SolverContainer lists in
mlo_dir_conv.cpp. Find enumerates exactly those two containers, so the solver was never
enumerated, never received a workspace size, and could never be selected — and because
TransposingSolver reports IsDynamic() == false, immediate mode could not reach it either.
This PR adds it to both lists beside its sibling TransposedConvWinoFuryRxS<2,3>, making the
NHWC transposed Winograd Rage path reachable for forward, backward-data and backward-weights.

JIRA ID : ALMIOPEN-2606

Risk Assessment

Risk level 3. The diff is two lines of solver-list wiring, one regression test and a CHANGELOG entry — no solver
logic, applicability, kernel or perf-db change. But it widens the Find candidate set on every
NHWC convolution on gfx942/gfx950/gfx120X (the solver's arch gate), which is a dispatch-
surface change rather than pure plumbing. Measured effect on gfx950: cold Find latency +16–25 %
on affected NHWC problems, zero steady-state execution change, zero workspace change, and the
selected winner is unchanged in 20/20 paired trials on every case measured.

Related

ASIC Coverage

Support-surface-expanding: a solver that could previously never be selected can now be
enumerated and selected. The solver's arch gate is a prefix test on gfx942, gfx950 and
gfx120 (src/solver/conv/conv_wino_rage_RxS.cpp:317-322), so the affected families are
gfx94X, gfx950 and gfx120X. gfx1250 is not covered — it does not match the gfx120
prefix, and the Rage suites skip there rather than fail (verified by spoofing
MIOPEN_DEVICE_ARCH=gfx1250).

Testing Summary

Baseline (origin/develop snapshot) vs. this branch, two fresh identically-configured Ninja
builds on MI355X (gfx950) / ROCm 10.1.0. Restricted to MIOpen source the two revisions differ
only by this fix, so upstream drift cannot be mistaken for it. All nine blocking exit criteria
passed.

  • Reproduction. Find enumerates 0 transposed Rage entries on the baseline and 2 here.
    Forcing the solver by name fails cleanly on the baseline with No suitable algorithm was
    found
    — no HIP error, no crash — and verifies numerically here in all 4 cases.
  • Regression test. The new AllRegisteredConvSolversAreFindReachable walks registry →
    Find-list, the direction the shipped guard did not cover. On the baseline it reports
    191 TransposedConvWinoRageRxS<2-3> as the only unreachable convolution solver (122
    registered vs. 121 reachable); here the two describe the same 122.
  • Functional. Unrestricted NHWC forward / backward-data / backward-weights: applicable,
    benchmarked, inserted into FindDb and numerically verified in all three directions.
  • Guardrails. MIOPEN_DEBUG_AMD_WINOGRAD_RAGE_RXS_F2X3=0 and MIOPEN_DEBUG_CONV_WINOGRAD=0
    each suppress the solver with a verifying fallback. NCHW is untouched — Not applicable, with
    identical winners on both builds. Reported workspace always covers the selected solution.
  • Regression sweep. All 25 Winograd unit-test binaries on both builds (135 passed, 110
    skipped, 25 disabled, 0 failed), differenced by individual test identity: zero differences.
  • Performance (record-only). 20 paired trials per case. Cold Find median +124 to +186 ms
    (+16.1 % to +24.9 %, disjoint IQRs) — the expected cost of benchmarking one more candidate on
    an empty cache. Execution median within +0.5 %, workspace delta 0 B. Winner unchanged in 20/20
    trials: Rage is benchmarked but never wins on gfx950, so Find correctly declines it.

The gfx950 baseline-vs-feature comparison above was run on the two-line fix alone, before the
regression test was added; the test is CPU-only and changes no product code.

Testing Checklist

  • Find-list/registry guard, existing direction - test_solver_find_list_registry - Devices: gfx950 (MI355X) - Status: Passed
  • Find-list/registry guard, new registry → Find-list direction - test_solver_find_list_registry --gtest_filter=*AllRegisteredConvSolversAreFindReachable* - Status: Pending (no MIOpen build on the authoring host; verified as a standalone probe on gfx950, where it fails on the baseline and passes with the fix)
  • Winograd Rage unit suites (filtered and full) - Devices: gfx950 (MI355X) - Status: Passed (11/11 and 36/36, 0 skipped)
  • Winograd family regression sweep, 25 binaries, baseline-differenced - Devices: gfx950 (MI355X) - Status: Passed (0 differences)
  • Find enumeration / forced-selection / kill-switch / NCHW-invariance checks - Devices: gfx950 (MI355X) - Status: Passed
  • Paired performance trials, 160 runs - Devices: gfx950 (MI355X) - Status: Passed (recorded, no regression in execution or workspace)
  • Multi-arch sweep - TheRock multi-arch CI - ASICs: gfx94X, gfx950, gfx120X - Status: Pending
  • PR CI - GitHub PR checks - Status: Pending

Flags / Guardrails

No new flag. The solver stays behind the two existing kill switches,
MIOPEN_DEBUG_AMD_WINOGRAD_RAGE_RXS_F2X3 and MIOPEN_DEBUG_CONV_WINOGRAD, both verified to
suppress it.

Technical Changes

  • projects/miopen/src/mlo_dir_conv.cpp: add TransposedConvWinoRageRxS<2, 3> to
    GetWindogradSolvers() (forward, backward-data) and GetWindogradWrWSolvers()
    (backward-weights), beside the existing TransposedConvWinoFuryRxS<2, 3>.
  • projects/miopen/test/gtest/solver_find_list_registry.cpp: add
    AllRegisteredConvSolversAreFindReachable, asserting every registered convolution solver
    appears in a Find list.
  • projects/miopen/CHANGELOG.md: record the fix under Resolved Issues.

TransposedConvWinoRageRxS<2,3> exists, is registered in the solver
registry, is explicitly instantiated and has unit tests, but was absent
from both SolverContainer lists in mlo_dir_conv.cpp. Find enumerates
exactly those two containers, so the solver was never enumerated, never
got a workspace size, and could never be selected.

Its sibling TransposedConvWinoFuryRxS<2,3> is present in both lists.
The shipped guard checks Find-list -> registry. The defect fixed in this
branch was the opposite direction: a solver registered in solver.cpp but
absent from every Find solver list in mlo_dir_conv.cpp, which Find can
never enumerate or select.

AllRegisteredConvSolversAreFindReachable asserts that direction. It fails
on the unpatched tree, reporting id 191 TransposedConvWinoRageRxS<2-3> as
the only unreachable convolution solver, and passes with the fix.
@therock-pr-bot

therock-pr-bot Bot commented Sep 10, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

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

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

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

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

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

@DarylHawkinsAMD
DarylHawkinsAMD marked this pull request as ready for review September 10, 2026 21:39
@DarylHawkinsAMD
DarylHawkinsAMD requested review from a team as code owners September 10, 2026 21:39

@averinevg averinevg left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@iq136boy iq136boy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@DarylHawkinsAMD
DarylHawkinsAMD merged commit 4084dbd into develop Sep 14, 2026
139 checks passed
@DarylHawkinsAMD
DarylHawkinsAMD deleted the users/dahawkin/register-transposed-wino-rage branch September 14, 2026 16:44
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

JonathanLichtnerAMD pushed a commit that referenced this pull request Oct 15, 2026
)

## Motivation

The code behind these changelog entries merged after the ROCm 10.1 snap,
so they did not ship in 10.1. They belong in the next release section,
not in the released section. This moves them and changes no text.

## Technical Details

Moves 2 entries to `MIOpen 3.6.2 for ROCm 10.2.0`, each under its
matching category heading:

- [#11949](#11949): [Conv]
Fixed `TransposedConvWinoRageRxS<2,3>` being unreachable: it was
registered in the solver ...
- Merge commit `4084dbd194` is not in the snap (GitHub compare status
`diverged`).
- 1 line(s) the PR added to `projects/miopen/src/mlo_dir_conv.cpp` are
missing from it at the pin, e.g.
`miopen::solver::conv::TransposedConvWinoRageRxS<2, 3>,`.
- [#11809](#11809): [Conv]
Relaxed the ASM-GTC NHWC large-tensor guard for forward and
backward-data. Those kernels ...
- Merge commit `5a28fa27a6` is not in the snap (GitHub compare status
`diverged`).
- 3 line(s) the PR added to
`projects/miopen/src/include/miopen/conv/asm_implicit_gemm.hpp` are
missing from it at the pin, e.g. `const auto group_count =
static_cast<std::size_t>(problem.GetGroupCount());`.

If an entry should stay where it is, say so in a comment and it will be
dropped from this PR.

## Test Plan

Check each merge commit against the ROCm 10.1 snap
(`release/therock-10.1`, `rocm-libraries` pin
`39d8d60365d2272a5366f76ccb3bfba24b219eaf`):

```
git fetch origin 39d8d60
git merge-base --is-ancestor 4084dbd 39d8d60; echo $?   # 1 means not in the snap
git merge-base --is-ancestor 5a28fa2 39d8d60; echo $?   # 1 means not in the snap
```

## Test Result

- Each merge commit is not an ancestor of the pin (compare status
`diverged` or `ahead`).
- The code each PR added is absent from the files at the pin (see
Technical Details).
- The patch applies cleanly to develop, and the non-blank lines of the
changelog are identical before and after, so this is a pure move.

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/rocm-libraries/blob/develop/CONTRIBUTING.md.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants