Repository navigation
[MIOpen] Sync hipconv v0.3.2 - #13016
Conversation
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>
hipconv v0.3.2 renamed the option, and CMake ignores the old name without a warning. The hipBLASLt paths stay off: the new option also defaults to OFF.
Add NHWC test lists that reach each kernel hipconv serves through ConvHipConv: direct_l1, direct, direct_wgrad, patch_embed, the grouped and grouped wgrad kernels at 4, 8, 16 and 32 channels per group, and the depthwise Toeplitz and Hankel kernels on gfx950; direct, the grouped kernels and the depthwise kernels on gfx1250. Each list is instantiated only on the architectures and directions whose kernels serve all of its layers, since the harness fails an inapplicable case. GenericSearch samples configs at random, so on a layer several kernels serve, Tunable(5) verifies whichever sample is fastest. The grouped stride-1 layers are such layers; their suites use Tunable(0), which runs only the default config, hipconv's top-ranked. Correct the reason the tf32 suites are gfx950-only: MIOpen enables tf32 compute on gfx942 and gfx95x alone, though hipconv's gfx1250 kernels serve it.
✅ All Checks Passed — Ready for Review
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
🎉 All checks passed! This PR is ready for review. |
SmokeConv3dDepth and SmokeDenseWrw declared gfx125X, but hipconv has no cdna5 kernel for their shapes, so IsApplicable fails every case there. Restrict both to gfx950. The fp16 dgrad of the 3-channels-per-group odd-channel case overflowed its reference: dgrad reduces over K/g*kh*kw = 576 terms against fprop's 27. K=16 keeps 3 channels per group and the same kernels at 36 terms. On MI455: 208 pass, 191 skip, 0 fail (was 37 fail). gfx950 unchanged.
|
The ConvHipConv unit tests include several layers that hipconv does not yet support on gfx1250, so The same commit adjusts one test for an overflow. MIOpen sizes the test data's range by the forward convolution's reduction depth, but dgrad reduces over a different depth, which can overflow fp16. The odd-channel grouped case now uses fewer output channels, which keeps it on the same kernels. With the commit, all ConvHipConv unit tests pass on MI455 (gfx1250) and gfx950. |
JonathanLichtnerAMD
left a comment
There was a problem hiding this comment.
LGTM. Just a minor NIT on the miopen changelog: we don't usually mention the jira tickets in the changelog or the code, e.g., ALMIOPEN-2624.
Fixed, thanks. |
Pre-commit check failed⛔ pre-commit failed Please run locally:
This repo uses |
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>
JIRA ID : ALMIOPEN-2361
Takes the vendored
src/hipconvsnapshot from v0.3.1 to v0.3.2. Per-version detail is in the shippedsrc/hipconv/CHANGELOG.md.Data types
ConvHipConv supports fp16, bf16 and TF32, and not fp32. This is unchanged from v0.2.7 and v0.3.1. hipconv emulates TF32 with three bf16 MFMAs on a (big, small) bf16 split of the stored fp32, so it needs no TF32 hardware, and its gfx950 and gfx1250 kernels both support it.
MIOpen decides when a problem is TF32:
MIOPEN_CONVOLUTION_ATTRIB_MATH_TYPEset tomiopenMathDefaultorMIOPEN_TF32_OVERRIDE, on a deviceIsTF32Supportedadmits. That list is gfx942 and gfx95x today, so through MIOpen, TF32 reaches hipconv on gfx950 and not yet on gfx1250.Additional TF32 support on gfx950 since v0.3.1:
direct_l1forward and backward-data, 2x2 to 5x5 filters (hipconv [rocrand] backport from release-staging-7.0 #226);depthwise_1d_toeplitzforward and backward-data (hipconv [rocprim] backport from release-staging-7.0 #223);direct_wgradbackward-weights at 2x2, 3x1, 3x3, 4x4 and 5x5 (hipconv [Discussion]: Cherry-pick from monorepo to original repositories #234).Kernels
patch_embed: patch-embedding convolutions (stride equal to the filter, no padding), forward, backward-data and backward-weights, fp16/bf16.depthwise_wgrad_hankel: depthwise backward-weights, fp16/bf16, 3x3 to 11x11 odd square, stride 1 or 2.direct: fixes a crash when groups x batch tiles exceeded the 65535 grid z limit (ALMIOPEN-2624), and an LDS race between a TDM load and another wave's reads; the race fix lifts the odd-channel backward-data restriction v0.3.1 added.directno longer claims depthwise layers (one channel per group).depthwise_wgrad_hankelsums its partitions in a fold kernel instead of with atomics: geometric mean +88% over fifteen shapes (hipconv [hipCUB] add memory to benchmark_device_batch_memcpy (#219) #227).Build
src/CMakeLists.txtrenamesHIPCONV_ENABLE_POINTWISEtoHIPCONV_ENABLE_EXPLICIT_GEMM, the name hipconv v0.3.2 gives the option, which now enables explicit-GEMM patch embedding as well as pointwise. CMake ignores the old name without a warning. Both explicit-GEMM paths stay off, since the new option also defaults to OFF.Downstream patches
On develop,
projects/miopen/src/hipconvdiffered from hipconv v0.3.1 only by #12406, which patchedsrc/arch/cdna4/direct/direct_wgrad/epilogue.h. hipconv v0.3.2 contains the same fix (hipconve1777ac9), so this sync keeps the file byte-identical.Validation
GetAdjustedConvolutionStride{H,W}hands hipconv a stride of 1 on an axis whose output extent is 1. They are 6 grouped stride-2 layers with a degenerate spatial extent, in forward and backward-data, and 2 patch-embedding layers with a 1x1 output, in all three directions.f58e0771, from which v0.3.2 differs in formatting, its changelog, host warning flags and one constexpr expression.clang-tidy over the consumer TU is clean with AMD LLVM 24.
ConvHipConv unit tests
unit_conv_solver_ConvHipConv.cppgains NHWC tests that cover every hipconv kernel availablethrough ConvHipConv. The layers come from hipconv's internal tests.
GetConvDenseTestCasesdirect_l1,direct(Fwd, Bwd);direct_wgrad(Wrw); tf32 ondirect_l1,direct_wgraddirect(Fwd, Bwd)GetConvDenseVerticalTestCasesdirect_l1,direct_wgradGetConvPatchEmbedTestCasespatch_embed(Fwd, Bwd, Wrw)GetConvOddChannelTestCasesdirect_l1,direct_wgraddirect(Fwd, Bwd)GetConvPointwiseTestCasesdirect1×1 (Fwd, Bwd)GetConvGroupedTestCasesgrouped,grouped_wgradat 4, 8, 16 and 32 channels per group; fp16, bf16, tf32grouped_multi_g,grouped_multi_g_wgradGetConvGroupedStride2TestCasesgroupedat 4, 16 and 32 channels per group (Fwd, Bwd)grouped_multi_g(Fwd, Bwd)GetConvGrouped8cStride2TestCasesgroupedat 8 channels per group (Fwd)grouped_multi_g(Fwd, Bwd)GetConvDepthwiseTestCasesdepthwise_1d_toeplitz(Fwd, Bwd; tf32 too),depthwise_wgrad_hankel(Wrw)Each list is instantiated only on the architectures and directions whose kernels support all of its
layers, since the harness fails an inapplicable case. The new lists are NHWC only: the NCHW
staging does not depend on the kernel, and the existing smoke lists cover it.
Tunable(0).GenericSearchshuffles every valid perf config and benchmarks the firstTunable(N), soTunable(5)verifies the fastest of five random configs. On a layer severalkernels support, which kernel that is varies from run to run. The grouped stride-1 layers are such
layers (the direct kernels support them too), as are the tf32 depthwise layers. Their suites use
Tunable(0): the sample is empty, so the search runs only the default config, which is hipconv'stop-ranked, and the grouped or depthwise kernel is the one verified. Every other list keeps
Tunable(5), since one kernel supports each of its layers and the sampling still exercises itsnon-default configs.
Testing. On gfx950 (MI355X): 399 tests, 375 pass and 24 skip (the gfx125X-only suites), the
same in each of five repeats. With
MIOPEN_LOG_LEVEL=5, each new case's finalGetSolutionnamesthe kernel it was meant to probe. On MI455 A0 (gfx1250): 208 pass and 191 skip (the gfx950-only suites), each case on its intended kernel; CI skips the gfx1250 cases, and hipconv's own suite also passed on MI455 A0
(Validation above).