Skip to content

[MIOpen] Sync hipconv v0.4.0 - #13213

Merged
amd-alavin merged 9 commits into
developfrom
users/amd-alavin/hipconv-v0.4.0
Oct 9, 2026
Merged

amd-alavin merged 9 commits into
developfrom
users/amd-alavin/hipconv-v0.4.0

Conversation

@amd-alavin

@amd-alavin amd-alavin commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

JIRA ID : ALMIOPEN-2361

Takes the vendored src/hipconv snapshot from v0.3.2 to v0.4.0 and fixes ConvHipConv's perf-db round trip, which broke SD2.1 fp16 batch 4 on gfx950 in the 10-02 nightly (ROCM-32185).

ConvHipConv perf-db records

#9680 made ConvHipConv's perf config the hipconv kernel label, e.g. direct[tile_size_k=256,tile_size_n=1,...]. PerfConfigBase deserializes field by field and splits on ,, so a stored label reloaded as direct[tile_size_k=256, matched no kernel, and the solver fell back to config 0. Under MIGraphX's --exhaustive-tune, execution then built config 0 against the workspace Find had reported for the tuned config and threw ConvHipConv: not enough workspace for direct kernel (conv_hipconv.cpp:702).

  • fix(miopen): serialize the ConvHipConv perf config whole: PerformanceConfigConvHipConv derives from PerfConfig and stores the label as one string.
  • fix(miopen): key ConvHipConv perf-db records to the hipconv minor version: a record is <hipconv config_version>:family[full description], e.g. v0.4:direct[tile_size_k=64,...,direction=fprop,type=fp16,...]. hipconv guarantees that a full description selects the same config only within a minor version, so a record from another minor version, an unstamped record, and any record in a build whose config_version() is unknown select nothing. The solver then falls back as for any invalid record (search, the AI picker, or hipconv's top-ranked config). Every ConvHipConv record written before this change is unstamped and is discarded once. The performance-log kernel name keeps the short label.

#9680's perf-config picker catalog (lgbm_pcfg.bin) holds 166 ConvHipConv labels in the old form. They never resolved, since every one contains a comma, and they will not match stamped records; they need regenerating against v0.4.0.

hipconv v0.4.0

Per-version detail is in the shipped src/hipconv/CHANGELOG.md.

Data types are unchanged: fp16, bf16 and TF32. projects/miopen/CHANGELOG.md gains the gfx1250 dense backward-weights kernel and the perf-db fix under MIOpen 3.6.2.

Downstream patches

None. On develop, projects/miopen/src/hipconv was byte-identical to hipconv v0.3.2.

Validation

  • hipconv's internal suite at v0.4.0: 65/65 on gfx950 (MI355X); 65/65 on MI455 A0 (gfx1250) with explicit GEMM off, including about 14,000 cases that run every kernel config against hipconv's test layers.
  • test_unit_conv_solver_ConvHipConv on gfx950 (MI355X), MIOpen built with clang 24 (ROCm 10.1) from this branch: 377 pass and 24 skip (the gfx125X-only suites), the same in two repeats. On MI455 A0 (gfx1250), built for gfx1250;gfx1250-strict: 210 pass and 191 skip (the gfx950-only suites), the same in two runs. New: CPU_UnitTestConvSolverConvHipConvPerfConfig_NONE.SerializeRoundTrip and GPU_UnitTestConvSolverConvHipConvPerfConfig_FP16.VersionStamp, which pass on both.
  • ROCM-32185 reproduction on gfx950, the 18 stride-1 SD2.1 batch-8 layers through Find 2.0 with ConvHipConv forced: exhaustive Find, save and load the solution, run on a second handle with the workspace Find reported. On develop it throws at conv_hipconv.cpp:702. With this branch, every layer passes on a fresh user perf db (one search each), again reloading those records (no search, no warning), and with the records restamped v0.3 (each rejected once with the "Invalid config" warning, re-searched, then passing).
  • clang-tidy over the consumer TU is clean (LLVM 21.1.8).

A find-db record also caches a workspace size and carries no hipconv version; that is unchanged here.

amd-alavin and others added 4 commits October 6, 2026 17:40
#9680 made the kernel label ("direct[tile_size_k=64,...]") ConvHipConv's
serialized perf config. PerfConfigBase deserializes field by field, splitting
on ',', so every config loaded from the perf DB or the perf-config picker
catalog came back as "direct[tile_size_k=64", matched no kernel, and fell
back to the default config.

When execution rebuilds the invoker from the perf DB, it then runs the
default config against the workspace Find reported for the tuned config, and
throws "ConvHipConv: not enough workspace for direct kernel" when the default
needs more (ROCM-32185: SD2.1 fp16 batch 4 on gfx950 under MIGraphX
--exhaustive-tune). Find 2.0 also re-ran the full search after every Find.

Derive from PerfConfig and serialize the label as one string. Records written
since #9680 store the full label and load unchanged.
Arches + files snapshot: see projects/miopen/src/hipconv/HIPCONV_VERSION

Co-authored-by: Andrew Lavin <andrew.lavin@amd.com>
Co-authored-by: Boris Prokopenko <boris.prokopenko@amd.com>
Co-authored-by: Carsten Uphoff <carsten.uphoff@amd.com>
Co-authored-by: Jin Zhou <jin.zhou@amd.com>
Co-authored-by: Petar Jovanovic <petar.jovanovic@amd.com>
Co-authored-by: Yingmao Lu <yingmao.lu@amd.com>
…sion

A full hipconv config description selects one config only within a hipconv
minor version, so a record written by another version may select a different
config or none. Store records as "<config_version>:family[full description]"
and match whole strings: a record from another minor version, an unstamped
record, and any record in a build whose config_version is "unknown" select
nothing, and the solver falls back to search or the default config.

Every record written before this change is unstamped and is discarded once.
The performance-log kernel name keeps the short label.
@therock-pr-bot

therock-pr-bot Bot commented Oct 6, 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

therock-pr-bot Bot commented Oct 6, 2026

Copy link
Copy Markdown

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

…ipConv.cpp

unit_conv_solver_*.cpp files hold RunTest smoke and full cases. Move
SerializeRoundTrip and VersionStamp next to the other per-solver perf-config
tests, as CPU_PerfConfig_ConvHipConv_NONE and GPU_PerfConfig_ConvHipConv_FP16.
… before enumerating

IsValid enumerated and ranked every config for the problem before comparing a
record against them. The perf-config picker from #9680 calls it once per
candidate in its bucket (up to 168 on gfx950), and no catalog entry carries a
version prefix, so a perf-db miss paid one enumeration per candidate before
falling back to config 0. Check the prefix first.
clang-tidy (modernize-use-starts-ends-with) rejects substr(0, n) == prefix.
@amd-alavin
amd-alavin requested a review from averinevg October 8, 2026 02:18

@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.

Thanks for addressing my comments! A few more notes inline.

VersionStamp checked only config 0's record, which is also the config a
record that matches nothing falls back to, and only that IsValid accepted
it. It now runs over every config of a dense 3x3 fprop layer (four on gfx950
and gfx1250): each record must resolve to its own index, and the record
under another version stamp, unknown, or no stamp must be rejected.
hipconv v0.4.0 adds a gfx1250 direct_wgrad kernel, which serves the dense
and vertical 3x1 Wrw shapes, so SmokeDenseWrw and the Wrw instances of
SmokeDense and SmokeDenseVertical no longer need to be gfx950-only.
@amd-alavin
amd-alavin requested a review from averinevg October 8, 2026 20:49

@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

@amd-alavin
amd-alavin merged commit 4ccd8f5 into develop Oct 9, 2026
97 checks passed
@amd-alavin
amd-alavin deleted the users/amd-alavin/hipconv-v0.4.0 branch October 9, 2026 16:23
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

mattwill-amd pushed a commit that referenced this pull request Oct 14, 2026
JIRA ID: ALMIOPEN-2913

## Motivation

Only 5 of the 30 commits that landed against `projects/miopen` since the
3.6.2 version bump (`2d20a8f775e`, #12211) were reflected in
`CHANGELOG.md`. The remaining 25 were reviewed and the 15
externally-visible ones documented, so the ROCm 10.2.0 section
accurately reflects what shipped.

14 entries added (two of the fixes share one entry), plus a
duplicate-heading fix.

### Added
* [Conv] NHWC (channel-last) support in the GEMM convolution solvers,
with new im2col/col2im kernels (#8693)
* [Conv] Cross-architecture AI heuristics for immediate mode
(`MIOPEN_FIND_MODE=2`), extending data-driven selection to Navi (#9680)
* [Conv] Missing 3x3 stride-1 pad-1 tile-28 CK depthwise backward-data
instance — ALMIOPEN-2423 (#11271)

### Changed
* [Conv] `ConvHipConv` held out of the AI perf-config picker until its
model is retrained — ALMIOPEN-2494 (#13337)
* [Conv] CK byte-span gate lifted for forward and backward-data, now
that CK fixes the 32-bit offset overflow itself — ALMIOPEN-2645 (#12396)

### Optimized
* [Conv] `ConvDirectNaiveConv`: per-direction kernel files, float
accumulators, spatial tiling, bfloat16 path — ALMIOPEN-2485/2486 (#7529)
* Kernel cache reuses cached SQLite connections instead of reopening per
load/store — ALMIOPEN-2598 (#11933)

### Resolved Issues
* [Conv] Incorrect backward-weights results from the gfx950 direct
kernel at `waves_q == 1` (missing cross-lane ordering on LDS staging
reuse) — ROCM-31508 (#12406)
* [Conv] Up to 8x slower hipconv `direct`/`direct_l1`/`direct_wgrad`
under ROCm 10.1's full AMDGCN LTO default; fixed with `-fno-offload-lto`
— ROCM-31334 (#12328)
* [Conv] gfx942 multi-node training throughput regression from backward
GEMM on small-batch 1x1 NHWC shapes — ROCM-31155 (#13232)
* [Conv] Invalid CK perf-config entries in the gfx1201 and gfx1100
system databases — ROCM-29026 (#12088, #12117)
* [BatchNorm] JIT compilation failures on non-GFX9 targets; Welford DPP
assembly restricted to GFX9 — ALMIOPEN-2838 (#12889)
* Out-of-bounds write in `miopenLoadSolution` when a serialized solution
carries more than three work dimensions — ROCM-31082 (#12242)
* `uint64_t` typedef conflict between `miopen_cstdint.hpp` and system
`<stdint.h>` on LP64 Linux (#12623)

## Technical Details

Single-file change to `projects/miopen/CHANGELOG.md`.

The 10.2.0 section had **two** `### Resolved Issues` headings. The two
entries under the trailing one (`ConvHipConv` gfx1250 crash/LDS race,
and the perf-db reload config loss) were merged into the first, and the
trailing heading removed — this accounts for the 2 removed bullets in
the diff. Section order is now `Resolved Issues → Added → Changed →
Optimized → Removed`, matching the 3.6.1 section. `Optimized` is new to
this release section.

Already documented, so untouched: #12874 (reduce int32 rejection),
#12000 / #13016 / #13213 (hipconv v0.3.1 / v0.3.2 / v0.4.0), #11761
(gfx803 removal).

## Notes

Deliberately excluded as not externally visible (10 commits):

* **hipDNN convolution forwarding (#12276)** —
`MIOPEN_ENABLE_HIPDNN_WRAPPER` defaults to `OFF`
(`projects/miopen/CMakeLists.txt:162`), so no shipped build compiles the
forwarding code and the runtime `MIOPEN_HIPDNN_FORWARDING` flag has
nothing to enable.
* **Test-only**: #13289 (gfx1250 depthwise test applicability,
ALMIOPEN-2841), #11610 (hipDNN parity harness), #12660 (Jenkins status
post).
* **Internal kernel-math cleanup, generated ISA unchanged**: #13312
(drop direct `__ocml` calls), #13190 (remove broken single-argument
`__half` overloads).
* **Add/revert pairs, net no change**: #12466 + #13066 (gfx1101 Navi
system DBs), #12496 + #12800 (gfx1250 in the dev image arch lists).

## Risk Assessment

Documentation-only change to a single Markdown file; no code, build, or
runtime impact.

**Risk Level: 🟢 Low**

## Test Plan

None required — Markdown-only. Verified that the entry topics are not
already documented elsewhere in the file, that no content was lost when
merging the duplicate heading, and that every new entry maps to a commit
in `2d20a8f775e..origin/develop -- projects/miopen`.

## Test Result

16 bullets added, 2 relocated — 14 net-new entries covering 15 commits.
Duplicate heading removed; section ordering matches 3.6.1.

Co-authored-by: Claude <noreply@anthropic.com>
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.

2 participants