Repository navigation
fix(miopen): always register CK grouped-conv solvers in host (ROCM-27359) - #9105
Merged
marbre merged 1 commit intoJul 6, 2026
Conversation
…M-27359) (#8988) JIRA ID: ROCM-27359 ## Motivation MIOpen's host `libMIOpen.so` is device-neutral and has no build-time Composable Kernel (CK) dependency: it never links CK device code and instead `dlopen`s per-arch `MIOpenCKGroupedConv_<arch>.so` plugins at runtime (`CkImplLibLoader`), degrading gracefully when a plugin is absent. However, host-side registration of the grouped-conv XDLOPS solvers in `mlo_dir_conv.cpp` was gated on `MIOPEN_USE_COMPOSABLEKERNEL`. When a build shard configured the host with CK=OFF (for example a CK-unsupported-only arch), the preprocessor stripped those solvers out of the solver registry entirely. Because the host ships as a single device-neutral `_generic` artifact, a CK-less `libMIOpen.so` could become the host for *every* arch and disable fast convolution everywhere -- including gfx942 -- producing a roughly 97% performance regression observed in a nightly (ROCM-27359). This PR fixes the regression in code so a CK-less host can no longer silently disable fast convolution. It is the code-side alternative to TheRock PR #6274, which tried to force `MIOPEN_USE_COMPOSABLEKERNEL=ON` at the build-wiring level and failed CI on CK-unsupported shards. With this change, TheRock no longer needs to force the flag on. ## Technical Details The `MIOPEN_USE_COMPOSABLEKERNEL` guard is removed from the host source paths so that solver registration and the dlopen prefetch are always compiled (still gated only on `MIOPEN_BACKEND_HIP`): - `src/mlo_dir_conv.cpp`: `GetImplicitGemmSolvers()` and `GetImplicitGemmWrWSolvers()` now register the grouped-conv XDLOPS solvers (`ConvHipImplicitGemmGroupFwdXdlops`, `ConvHipImplicitGemmGroupBwdXdlops`, `ConvHipImplicitGemm3DGroupFwdXdlops`, `ConvHipImplicitGemm3DGroupBwdXdlops`, `ConvHipImplicitGemmGroupWrwXdlops`, `ConvHipImplicitGemm3DGroupWrwXdlops`) under `#if MIOPEN_BACKEND_HIP` alone, dropping the `&& MIOPEN_USE_COMPOSABLEKERNEL` condition. - `src/hip/handlehip.cpp`: the `<miopen/solver/ck_impl_lib_loader.hpp>` include and the `PrefetchComposableKernel()` body (`CkImplLibLoader::Get(device_name)`) are now unconditional; the no-op `#else` branch is removed. - `src/include/miopen/conv/solvers.hpp`: removes a now-empty `#if MIOPEN_BACKEND_HIP && MIOPEN_USE_COMPOSABLEKERNEL` block and its comment. This is safe to compile and link with or without a CK package present: the solver classes are declared unconditionally in `solvers.hpp`, their implementations live in the unconditional MIOpen source list, and they link with NO CK headers -- they only use the runtime dlopen loader. `MIOPEN_USE_COMPOSABLEKERNEL` now only gates the per-arch CK plugin build (`src/ck_impl`, itself arch-gated) and the CK-header-dependent gtest. `CMakeLists.txt`: the comment above the `MIOPEN_USE_COMPOSABLEKERNEL` option is expanded to document this decoupling and why no per-arch build gating of the flag is needed. ## Risk Assessment **Risk Level: 🟡 Medium** Small diff (net 4 files, +14/-17) but it touches the convolution solver registry -- a foundational MIOpen path whose regression is exactly what caused ROCM-27359. The change direction is corrective (it restores solvers that should always be present), which bounds the risk. ### Impacted Components - MIOpen convolution solver registration (`src/mlo_dir_conv.cpp`) -- grouped-conv XDLOPS Fwd/Bwd/WrW solvers, 2D and 3D. - MIOpen HIP handle initialization / CK plugin prefetch (`src/hip/handlehip.cpp`). - Public/internal solver declarations (`src/include/miopen/conv/solvers.hpp`). - Build configuration (`CMakeLists.txt`) -- documentation only for the CK option. - Indirectly: TheRock packaging (PR #6274 becomes unnecessary) and every arch served by the device-neutral host artifact. ### Potential Side Effects - On a build with no CK plugin available at runtime, these solvers are now always registered; they must continue to degrade gracefully (return not-applicable / fall through) via the dlopen loader rather than failing. This is the intended and pre-existing runtime behavior of the loader, but reviewers should confirm no code path assumed the solvers were compiled out. - `handlehip.cpp` now always calls the CK prefetch during handle construction; on CK-less builds this must remain a benign no-op (loader returns absence). ### Mitigation Steps - The runtime loader (`CkImplLibLoader`) already handles absent plugins, so the always-registered solvers are inert without a plugin. - Revert plan: the change is self-contained to the listed host source files plus a CMake comment and can be reverted if a regression surfaces. - Recommended: validate a CK-less host build still resolves fast convolution on gfx942 (the ROCM-27359 repro) and confirm CK-enabled builds are unchanged. ## Test Plan - Configure and build the MIOpen host with `MIOPEN_USE_COMPOSABLEKERNEL=OFF` and confirm it compiles and links cleanly (the grouped-conv XDLOPS solvers are now always compiled and must link with no CK headers). - Cross-artifact repro of ROCM-27359: build the host with `MIOPEN_USE_COMPOSABLEKERNEL=OFF`, then run a grouped convolution on gfx942 with the per-arch CK plugin supplied via `MIOPEN_CK_LIB_PATH`; confirm `ConvHipImplicitGemmGroupFwdXdlops` is registered, the plugin loads, and the fast path is selected (before this fix the solver was absent from the registry). - Configure and build with `MIOPEN_USE_COMPOSABLEKERNEL=ON` with a CK package present: verify no behavior change and that CK-enabled builds and tests are unaffected. No new automated tests are added; the fix is a build/registration change validated through existing build-matrix configurations and the runtime repro above. ## Test Result To be confirmed by CI across the CK-supported and CK-unsupported build shards, plus the local cross-artifact repro. Expected outcome: the host library builds and links with CK ON or OFF and with or without a CK package present; grouped-conv XDLOPS solvers are always registered so a CK-less device-neutral host no longer disables fast convolution (resolving the ROCM-27359 ~97% regression); CK-enabled builds are unchanged. - [ ] multiarch sweep passes: https://github.com/ROCm/TheRock/actions/runs/28488014242 - [ ] CI passes ## Backport note (release/therock-7.14) Cherry-picked from c534dd1. The `src/hip/handlehip.cpp` hunk from the original PR was dropped during the backport: this branch does not contain the `PrefetchComposableKernel()` helper or the `<miopen/solver/ck_impl_lib_loader.hpp>` include that the original commit modified (that CK-prefetch mechanism was never introduced on this line and is absent from the base branch), so there was nothing to unguard. The dropped hunk was only a startup-latency optimization and is not part of the ROCM-27359 regression fix. The functional fix is unchanged and fully present: the grouped-conv XDLOPS solvers are always registered (`src/mlo_dir_conv.cpp`, gated on `MIOPEN_BACKEND_HIP` alone), the now-empty guard block is removed from `src/include/miopen/conv/solvers.hpp`, and the `CMakeLists.txt` documentation is updated. Note the CMake comment still references handlehip.cpp prefetch being compiled unconditionally, which does not apply to this branch. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
marbre
approved these changes
Jul 6, 2026
marbre
left a comment
Member
There was a problem hiding this comment.
LGTM. Excluding the original changes to projects/miopen/src/hip/handlehip.cpp make sense. Approving on behalf of dev ops.
BrianHarrisonAMD
approved these changes
Jul 6, 2026
astyrrian1
approved these changes
Jul 6, 2026
marbre
deleted the
users/jlichtne/ROCM-27359-remove-host-side-ck-disable-flag-cherry-pick
branch
July 6, 2026 22:21
marbre
added a commit
to ROCm/TheRock
that referenced
this pull request
Jul 7, 2026
Bumps rocm-libraries and rocm-systems to include the following cherry-picks: * ROCm/rocm-libraries#9105 * ROCm/rocm-libraries#9110 * ROCm/rocm-systems#8170
JonathanLichtnerAMD
added a commit
that referenced
this pull request
Jul 15, 2026
JIRA ID: ALMIOPEN-2312 ## Motivation `projects/miopen/CHANGELOG.md`'s "MIOpen 3.5.2 for ROCm 7.14.0" section only had 4 entries, but 68 commits touched `projects/miopen` between the `therock-7.13` tag and `release/therock-7.14`. Auditing that commit range (via `git log --oneline therock-7.13..remotes/origin/release/therock-7.14 -- projects/miopen`) found 18 additional user-facing changes — new gfx950/gfx1250 architecture support, SystemDB refreshes, AI-heuristic improvements, several correctness/crash bug fixes, and OpenCL solver removals — that were never documented. This PR backfills those entries so the 7.14.0 changelog accurately reflects what shipped. ## Technical Details Added 24 lines to the existing "MIOpen 3.5.2 for ROCm 7.14.0" section of `projects/miopen/CHANGELOG.md`, merging new bullets into the existing `Added`/`Changed`/`Resolved Issues` sub-sections (no new entries were duplicated or reworded) and adding a new `Removed` sub-section for solver removals. No existing content was altered. Each new bullet was traced to a specific commit + PR: | Entry | Commit | PR | |---|---|---| | Added gfx950 (MI350X/MI355X) SystemDB | `60d8e1e5175` | #8081 | | Added initial gfx1250 architecture support | `d3f057b2f22` | #7587 | | Added JSON-formatted performance logs (`MIOPEN_PERFORMANCE_LOGS`) | `ffbde871260` | #5736 | | Refreshed gfx942 SystemDB | `faf754a9ef6` | #8174 | | Refined 2D AI heuristics (Tunanet/KTN) for gfx942 & gfx950 | `c24ad415884` | #7109 | | Fixed thread-safety issue in AI-heuristic model caches | `cd957402309` | #9149 (cherry-pick of #9131) | | Fixed CK grouped-conv solvers not always registered in host | `c971d54d7ae` | #9105 | | Fixed legacy CK buffer resource descriptor (gfx1250) | `1dee011b8b0` | #8758 | | Removed stale gfx942 SystemDB naive-solver entries | `f42668f6316` | #7028 | | Fixed softmax NaN when `beta=0` | `fb8157c8512` | #8426 | | Fixed BN backward spatial variant 1 vectorized memory access bug | `1351a7e8b10` | #6765 | | Fixed use-after-free of `kernel_id` in `InitInvokerFactoryNCHW` | `394ebffb50d` | #7922 | | Fixed SoftMarginLoss forward reduce-kernel kernarg corruption | `1e65a31570f` | #8065, #8135 | | Fixed build with `MIOPEN_ENABLE_AI_KERNEL_TUNING=OFF` | `e6288c3ca61` | #6800 | | Fixed install RPATH for MIOpenDriver / CK backend libs | `ef8a4cf2f93` | #7485 | | Fixed dangling lambda causing log-level 6 segfault | `190f9791737` | #6953 | | Enabled Windows long-path support for `addkernels` | `1f9d69724cf` | #7387 | | Removed OCL solver `ConvOclDirectFwd1x1` | `126c720cadd` | #6491 | | Removed OCL solver `conv_ocl_dir2Dfwdgen` | `2d499ddf709` | #6633 | | Removed OCL solvers `ConvOclBwdWrW1x1`, `ConvOclBwdWrW2` | `b5d43a33dc6` | #6859 | | Removed OCL solvers `OpTensorLeadingOnes`, `Op5dTensorGeneric` | `8642c83b7de` | #7347 | The remaining ~40 commits in the 68-commit range were deliberately excluded (tests/CI/build/docs/internal refactors/1:1 OCL→HIP swaps with no behavior change, per each commit's own stated scope) and are not represented here. ## Risk Assessment **Risk Level: 🟢 Low** This is a documentation-only change to a single Markdown file. No code, build configuration, tests, or public API surface is touched. ### Impacted Components - `projects/miopen/CHANGELOG.md` only. ### Potential Side Effects None expected. This does not change any runtime behavior; it only documents changes that already shipped on `release/therock-7.14`. ### Mitigation Steps None required beyond normal review — a reviewer familiar with the 7.14 release can spot-check a few entries against the linked commit hashes/PR numbers for accuracy. ## Test Plan Not applicable (no code changes). Verification was done by cross-referencing every added bullet against a specific commit hash and PR number pulled from `git log`, and by diffing the edited section against `git show remotes/origin/release/therock-7.14:projects/miopen/CHANGELOG.md` to confirm no existing entry was duplicated or altered. ## Test Result All 21 new bullets map 1:1 to a real commit/PR in the `therock-7.13..release/therock-7.14` range for `projects/miopen`. No pre-existing changelog text was modified — `git diff` shows a pure addition (24 insertions, 0 deletions). Two borderline commits (`a67834413e8` "Standalone build", `7c7cd5774a9` "enable warnings about deprecated API/features") were deliberately left out pending author judgment on whether they warrant an entry.
pschang-phy
pushed a commit
that referenced
this pull request
Jul 17, 2026
JIRA ID: ALMIOPEN-2312 ## Motivation `projects/miopen/CHANGELOG.md`'s "MIOpen 3.5.2 for ROCm 7.14.0" section only had 4 entries, but 68 commits touched `projects/miopen` between the `therock-7.13` tag and `release/therock-7.14`. Auditing that commit range (via `git log --oneline therock-7.13..remotes/origin/release/therock-7.14 -- projects/miopen`) found 18 additional user-facing changes — new gfx950/gfx1250 architecture support, SystemDB refreshes, AI-heuristic improvements, several correctness/crash bug fixes, and OpenCL solver removals — that were never documented. This PR backfills those entries so the 7.14.0 changelog accurately reflects what shipped. ## Technical Details Added 24 lines to the existing "MIOpen 3.5.2 for ROCm 7.14.0" section of `projects/miopen/CHANGELOG.md`, merging new bullets into the existing `Added`/`Changed`/`Resolved Issues` sub-sections (no new entries were duplicated or reworded) and adding a new `Removed` sub-section for solver removals. No existing content was altered. Each new bullet was traced to a specific commit + PR: | Entry | Commit | PR | |---|---|---| | Added gfx950 (MI350X/MI355X) SystemDB | `60d8e1e5175` | #8081 | | Added initial gfx1250 architecture support | `d3f057b2f22` | #7587 | | Added JSON-formatted performance logs (`MIOPEN_PERFORMANCE_LOGS`) | `ffbde871260` | #5736 | | Refreshed gfx942 SystemDB | `faf754a9ef6` | #8174 | | Refined 2D AI heuristics (Tunanet/KTN) for gfx942 & gfx950 | `c24ad415884` | #7109 | | Fixed thread-safety issue in AI-heuristic model caches | `cd957402309` | #9149 (cherry-pick of #9131) | | Fixed CK grouped-conv solvers not always registered in host | `c971d54d7ae` | #9105 | | Fixed legacy CK buffer resource descriptor (gfx1250) | `1dee011b8b0` | #8758 | | Removed stale gfx942 SystemDB naive-solver entries | `f42668f6316` | #7028 | | Fixed softmax NaN when `beta=0` | `fb8157c8512` | #8426 | | Fixed BN backward spatial variant 1 vectorized memory access bug | `1351a7e8b10` | #6765 | | Fixed use-after-free of `kernel_id` in `InitInvokerFactoryNCHW` | `394ebffb50d` | #7922 | | Fixed SoftMarginLoss forward reduce-kernel kernarg corruption | `1e65a31570f` | #8065, #8135 | | Fixed build with `MIOPEN_ENABLE_AI_KERNEL_TUNING=OFF` | `e6288c3ca61` | #6800 | | Fixed install RPATH for MIOpenDriver / CK backend libs | `ef8a4cf2f93` | #7485 | | Fixed dangling lambda causing log-level 6 segfault | `190f9791737` | #6953 | | Enabled Windows long-path support for `addkernels` | `1f9d69724cf` | #7387 | | Removed OCL solver `ConvOclDirectFwd1x1` | `126c720cadd` | #6491 | | Removed OCL solver `conv_ocl_dir2Dfwdgen` | `2d499ddf709` | #6633 | | Removed OCL solvers `ConvOclBwdWrW1x1`, `ConvOclBwdWrW2` | `b5d43a33dc6` | #6859 | | Removed OCL solvers `OpTensorLeadingOnes`, `Op5dTensorGeneric` | `8642c83b7de` | #7347 | The remaining ~40 commits in the 68-commit range were deliberately excluded (tests/CI/build/docs/internal refactors/1:1 OCL→HIP swaps with no behavior change, per each commit's own stated scope) and are not represented here. ## Risk Assessment **Risk Level: 🟢 Low** This is a documentation-only change to a single Markdown file. No code, build configuration, tests, or public API surface is touched. ### Impacted Components - `projects/miopen/CHANGELOG.md` only. ### Potential Side Effects None expected. This does not change any runtime behavior; it only documents changes that already shipped on `release/therock-7.14`. ### Mitigation Steps None required beyond normal review — a reviewer familiar with the 7.14 release can spot-check a few entries against the linked commit hashes/PR numbers for accuracy. ## Test Plan Not applicable (no code changes). Verification was done by cross-referencing every added bullet against a specific commit hash and PR number pulled from `git log`, and by diffing the edited section against `git show remotes/origin/release/therock-7.14:projects/miopen/CHANGELOG.md` to confirm no existing entry was duplicated or altered. ## Test Result All 21 new bullets map 1:1 to a real commit/PR in the `therock-7.13..release/therock-7.14` range for `projects/miopen`. No pre-existing changelog text was modified — `git diff` shows a pure addition (24 insertions, 0 deletions). Two borderline commits (`a67834413e8` "Standalone build", `7c7cd5774a9` "enable warnings about deprecated API/features") were deliberately left out pending author judgment on whether they warrant an entry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JIRA ID: ROCM-27359
Motivation
MIOpen's host
libMIOpen.sois device-neutral and has no build-time Composable Kernel (CK) dependency: it never links CK device code and insteaddlopens per-archMIOpenCKGroupedConv_<arch>.soplugins at runtime (CkImplLibLoader), degrading gracefully when a plugin is absent.However, host-side registration of the grouped-conv XDLOPS solvers in
mlo_dir_conv.cppwas gated onMIOPEN_USE_COMPOSABLEKERNEL. When a build shard configured the host withMIOPEN_USE_COMPOSABLEKERNEL=OFF(for example a CK-unsupported-only arch), the preprocessor stripped those solvers out of the solver registry entirely. Because the host ships as a single device-neutral_genericartifact, a CK-lesslibMIOpen.socould become the host for every arch and disable fast convolution everywhere — including gfx942 — producing a roughly 97% performance regression observed in a nightly (ROCM-27359).This is a backport cherry-pick of upstream commit c534dd1 (PR #8988) onto
release/therock-7.14. The functional fix — always registering grouped-conv XDLOPS solvers underMIOPEN_BACKEND_HIPalone — is fully present. See the Mitigation section for the one hunk that was intentionally dropped during backport.Technical Details
The
MIOPEN_USE_COMPOSABLEKERNELguard is removed from the host solver-registration paths so that the grouped-conv XDLOPS solvers are always compiled (still gated only onMIOPEN_BACKEND_HIP):projects/miopen/src/mlo_dir_conv.cpp:GetImplicitGemmSolvers()andGetImplicitGemmWrWSolvers()now register the grouped-conv XDLOPS solvers (ConvHipImplicitGemmGroupFwdXdlops,ConvHipImplicitGemmGroupBwdXdlops,ConvHipImplicitGemm3DGroupFwdXdlops,ConvHipImplicitGemm3DGroupBwdXdlops,ConvHipImplicitGemmGroupWrwXdlops,ConvHipImplicitGemm3DGroupWrwXdlops) under#if MIOPEN_BACKEND_HIPalone, dropping the&& MIOPEN_USE_COMPOSABLEKERNELcondition.projects/miopen/src/include/miopen/conv/solvers.hpp: removes a now-empty#if MIOPEN_BACKEND_HIP && MIOPEN_USE_COMPOSABLEKERNELblock and its associated comment (the block contained no declarations — it was dead code).projects/miopen/CMakeLists.txt: the comment above theMIOPEN_USE_COMPOSABLEKERNELoption is expanded to document this decoupling and explain that the flag now only gates the per-arch CK plugin build (src/ck_impl) and the CK-header-dependent gtest; the host builds cleanly regardless.This is safe to compile and link with or without a CK package present: the solver classes are declared unconditionally in
solvers.hpp, their implementations live in the unconditional MIOpen source list, and they link with no CK headers — they only use the runtime dlopen loader.MIOPEN_USE_COMPOSABLEKERNELnow only gates the per-arch CK plugin build and CK-header-dependent tests.Backport note: The
src/hip/handlehip.cpphunk from the original upstream PR was intentionally dropped during this cherry-pick. Therelease/therock-7.14base branch does not contain thePrefetchComposableKernel()helper or the<miopen/solver/ck_impl_lib_loader.hpp>include that the original commit modified — the CK-prefetch mechanism was never introduced on this branch. The dropped hunk was a startup-latency optimization only and is not part of the ROCM-27359 regression fix. The net diff therefore touches 3 files instead of the original 4.Risk Assessment
Risk Level: 🟡 Medium
The diff is small (3 files, +14/-11 lines net) but touches the convolution solver registry — a foundational MIOpen path whose misregistration is exactly what caused the ~97% regression in ROCM-27359. The change direction is corrective (restoring solvers that should always be present), which bounds the risk. The dropped handlehip.cpp hunk further reduces the surface relative to the upstream PR.
Impacted Components
src/mlo_dir_conv.cpp) — grouped-conv XDLOPS Fwd/Bwd/WrW solvers, 2D and 3D.src/include/miopen/conv/solvers.hpp) — empty guard block removed.CMakeLists.txt) — documentation only; no logic change.Potential Side Effects
CkImplLibLoader, but reviewers should confirm no code path assumed the solvers were compiled out on CK-less builds.MIOPEN_USE_COMPOSABLEKERNEL=ON) should be completely unaffected, since the guards being removed were already satisfied in that configuration.Mitigation Steps
CkImplLibLoader) already handles absent plugins gracefully, so the always-registered solvers are inert when no plugin is present.handlehip.cppCK-prefetch hunk was not backported because it does not exist on this branch — this eliminates one change surface and one potential regression vector relative to the upstream PR.MIOPEN_USE_COMPOSABLEKERNEL=OFFhost build still resolves fast convolution on gfx942 (the ROCM-27359 repro) and confirmMIOPEN_USE_COMPOSABLEKERNEL=ONbuilds are unchanged.Test Plan
MIOPEN_USE_COMPOSABLEKERNEL=OFFand confirm it compiles and links cleanly (the grouped-conv XDLOPS solvers are now unconditionally compiled and must link with no CK headers).MIOPEN_USE_COMPOSABLEKERNEL=OFF, then run a grouped convolution on gfx942 with the per-arch CK plugin supplied viaMIOPEN_CK_LIB_PATH; confirmConvHipImplicitGemmGroupFwdXdlopsis registered, the plugin loads, and the fast path is selected (before this fix the solver was absent from the registry).MIOPEN_USE_COMPOSABLEKERNEL=ONwith a CK package present and verify no behavior change.Test Result
To be confirmed by CI across the CK-supported and CK-unsupported build shards on
release/therock-7.14. Expected outcome: the host library builds and links with CK ON or OFF; grouped-conv XDLOPS solvers are always registered so a CK-less device-neutral host no longer disables fast convolution (resolving the ROCM-27359 ~97% regression); CK-enabled builds are unchanged.