Repository navigation
feat(rocke/wgrad): add group merging (Gm) for depthwise grouped conv - #12359
Conversation
✅ 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. |
90817cc to
064ed2a
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
There are a few concrete correctness/documentation issues in the updated code/comments (including a snap/clamp ordering bug in grouped split-K selection) that should be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 4
Open (4)
What changed in this PR
This PR adds group merging (Gm) support for depthwise grouped conv wgrad in Rocke by introducing WgradConvSpec.group_merge, widening loader geometry via a synthetic “merged problem”, masking off-diagonal group pairs in epilogues, and extending the benchmark sweep and correctness/IR tests to cover merged configurations.
Changes:
- Introduces
group_mergeintoWgradConvSpec, adds merged-vs-true dimension separation (grid_*vswg_*), and implements diagonal-masked stores for merged tiles (direct/CShuffle and two-stage workspace store). - Updates split-K auto-selection to be groups-aware (base grid includes groups, occupancy target scales with block size, optional XCD-alignment snap) and threads the new parameters through dispatch/tests.
- Extends benchmarks and tests to sweep/validate merged configurations, including host-side gate agreement and GPU numerics with additional coverage guards.
| File | Description |
|---|---|
| dnn-providers/hip-kernel-provider/rocke/platform/tests/instances/rocke_ir_parity_harness.py | Updates IR parity harness commentary for the grouped wgrad instance. |
| dnn-providers/hip-kernel-provider/rocke/platform/tests/dispatch/dispatch_tests/gemm/test_split_k_wgrad.py | New unit tests pinning ungrouped split-K behavior and validating grouped-path invariants. |
| dnn-providers/hip-kernel-provider/rocke/platform/python/rocke/helpers/split_k.py | Extends select_split_k_wgrad with grouped-branch logic (groups + block_size + XCD snap). |
| dnn-providers/hip-kernel-provider/rocke/library/tests/test_conv_wgrad_correctness.py | Threads groups/block_size into split-K selection; adds group-merge correctness + gate tests and coverage guards. |
| dnn-providers/hip-kernel-provider/rocke/library/tests/dispatch/test_grouped_conv_wgrad_dispatch.py | Updates expected grid derivation to use groups-aware split-K selection inputs. |
| dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_implicit_gemm_wgrad.py | Adds group_merge, merged geometry properties, merged epilogue addressing/masking, and shared gate. |
| dnn-providers/hip-kernel-provider/rocke/library/kernels/common/conv_implicit_gemm_wgrad_two_stage.py | Ensures split-K auto-resolution uses grid_groups and block size (merged-aware). |
| dnn-providers/hip-kernel-provider/rocke/library/dispatch/grouped_convolution.py | Forces epilogue=default for two-stage and passes groups/block_size into split-K selection. |
| dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/benchmark_implicit_gemm_conv.py | Adds WgradCombo with group_merge, extends sweep space for depthwise merged instances, and updates reporting/pruning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
0bd24d2 to
b742a9b
Compare
select_split_k_wgrad sized the degree from one group's GEMM, so a grouped wgrad with a 1-2 tile base grid asked for hundreds of slices and got clamped by the gridDim.z limit. Add the groups factor, and size the occupancy target in waves rather than a fixed blocks-per-CU constant -- the groups factor alone collapses the degree to 1, which is worse. Snap the degree so base_grid*split_k clears the XCD count: consecutive conv groups sit that far apart in flat workgroup id, and misalignment scatters groups that share a cache line across XCD L2s. groups <= 1 keeps the previous expression verbatim, so dgrad and every ungrouped caller are unchanged.
A depthwise group has cpg == kpg == 1, so its per-group GEMM operands are single channels of an NHWC tensor: the free axis is one element wide and every load degenerates to a scalar. Merging Gm adjacent groups into one workgroup makes the free axis a run of Gm consecutive channels, so the same work issues as vector loads and one (n, ho, wo) address decode is amortised over Gm channels. On a depthwise bf16 shape the main-loop load collapses from 72 buffer_load_ushort to 9 buffer_load_dwordx4, and the K-outer store side from 72 ds_write_b16 to 9. The merged tile computes a Gm x Gm block of group pairs; only the diagonal is wanted, and the two-stage workspace epilogue masks the rest by folding an equality test into the store predicate it already has. wg_M/wg_N stay the true per-group dims that size dW and the workspace; new grid_M/grid_N/ grid_groups are what the tile covers. Taking the accumulator bound from the true dims instead of the merged ones silently drops (1 - 1/Gm) of the dW rows, so the correctness suite gains row and column coverage guards. Gated to depthwise, MFMA, power-of-two Gm dividing groups, and the two-stage path -- the other epilogues do not carry the mask yet. group_merge defaults to 1, so every existing kernel is byte-identical and keeps its name. No C++ mirror is needed: that engine rejects groups != 1 for wgrad outright. Also drops three docstrings claiming is_valid_wgrad_spec rejects Gm>1; there was no such field to reject.
Group merging and split-K are alternative ways to spend the same parallelism, so gate them as mutually exclusive: group_merge > 1 now requires split_k == 1 and rejects two_stage. Merged instances store dW straight from the tile, so the direct and CShuffle epilogues gained the block-diagonal mask -- it rides in the addr_fn valid predicate the store guard already ANDs, and the unreachable merged branch in the two-stage workspace epilogue is removed. The wgrad benchmark sweeps both families for a depthwise problem and ranks them together, with no new flag: merged combos at split_k=1 alongside the split-K instances. The combo tuple becomes a NamedTuple with a defaulted group_merge field first -- four sites unpack it positionally, which is how a stale unpack once reached only the odd-cpg leg that CI never runs. Adds coverage for the single-stage merged path over both epilogues and all degrees, the grid_groups == 1 edge case, and the exclusion itself. The merged cases assert they did not skip: _run_one reports an unbuildable spec as ok=True, which made the first version pass vacuously.
Merging and split-K fix different things and compose: merging widens the depthwise load from scalar to a full channel run but divides the grid by Gm, and split-K multiplies it back. Gating them as alternatives left the best depthwise configuration unreachable. group_merge > 1 may now use split_k > 1 via two-stage. The merged branch returns to the workspace-store epilogue, this time on the shared coord splitter the direct and CShuffle stores already use. The packed-atomic split-K epilogues stay rejected: they have no way to drop an off-diagonal group pair and would accumulate into a live dW element rather than skip the store. Depthwise no longer sweeps single-stage degrees. split_k in (0, 1) is the only route to the direct-store and runtime-atomic bodies, and with one workgroup per merged group neither can fill the device -- the reduction degree is where the parallelism comes from on these shapes. The merged family also skips the 'basic' pipeline. Merging shrinks the grid, the groups-aware selector answers with a deeper split_k, and the shorter K-slice drops the iteration count back under the unroll cap, so the merged combos are exactly the ones that would fully unroll and cost minutes each to compile. Ranked output and the timed rows carry a gm tag; without it two entries differing only in Gm are indistinguishable and collide in the prune key.
The cap was applied after the snap, so a shallow wg_K or a large group count against the gridDim.z limit pulled the degree straight back off a multiple of the step and undid the alignment. Clamp first, then snap, and keep the legal degree when the cap is below one full step and alignment is unreachable. The test matrix missed it because every shape had a deep wg_K; adds shallow-K shapes and pins the ordering directly. Also: drop a stale "mutually exclusive" comment left over from the version that gated merging and split-K apart, correct two docstrings still claiming group merging is unimplemented, move the host-side group-merge gate tests into their own file, and name the skip guard _assert_case_ran so it reads as deliberate rather than debug residue.
b742a9b to
23bb138
Compare
TheRock Submodule Bump ActivityNewest first
|
…grad on gfx950 (#12951) ## Summary Depthwise backward-weight convolution has had merged-group (`Gm`) support in the kernel since #12359, but nothing ever asked for it. `group_merge` defaults to 1 and `dispatch_conv_grouped` never set it, so the only ways to reach a merged kernel were to hand-construct a spec in a test or pass `--group-merge N` on the benchmark CLI. Every production depthwise wgrad ran unmerged. This PR adds the selection policy — `_wgrad_merge_degree()` — plus the plumbing that lets a non-default degree survive the trip from spec selection to launch. No kernel capability is added or changed. Why it matters: in wgrad the per-group GEMM is `kpg x (Z*Y*X*cpg) x (N*Ho*Wo)`, so depthwise (`cpg == kpg == 1`) makes the free axis one element wide and every B load scalar. Merging makes that run `Gm` elements long — `Gm` *is* the load vector width. ## What changed `library/dispatch/grouped_convolution.py`: - **`_wgrad_merge_degree()`** — the policy. Picks the largest degree the kernel's own gate admits at the tile that already ships. - **`ConvGroupedSpec.group_merge`** field, forwarded by `to_wgrad_spec()`, and emitted by `kernel_name()` as `gm{N}` — merging changes the GEMM, so merged and unmerged kernels must not collide on the compile-cache key. - **`_wgrad_grid()`** — z extent becomes `(groups / Gm) * split_k`; without this we would launch `Gm x` too many CTAs. - **`_resolve_wgrad_split_k()`** — split-K is chosen from GEMM dims that merging scales, and merged + `split_k > 1` is routed to two-stage. The workspace sizing is deliberately left **unmerged**, mirroring `wgrad_two_stage_workspace_nbytes`: merging changes how many CTAs write `dW`, not how much `dW` there is. Scaling it by `gm` would overstate the allocation quadratically and start refusing two-stage on problems that fit. ### There is no fitted constant, and that is the result The forward selector (#12793) is a fitted closed form with eight constants. Backward-weight ended up with none. Forward puts `Gm` on GemmN+GemmK, trading vector width against a working set growing in `Y*X*Gm` — a genuine interior optimum. Wgrad puts `Gm` on GemmM+GemmN and the gate caps it at `Z*Y*X*cpg*Gm <= tile_n`; with the tile fixed, the admissible set is two or three degrees wide and its top is nearly always best. Three richer policies were fitted and rejected; `score_policy.py` re-derives the rejection from the raw sweeps. ## Measured evidence Relative ratios only, per `platform/AGENTS.md` §Compliance. No absolute latency, throughput, or achieved-FLOP figure appears here or in any commit message. - **Hardware target:** gfx950. - **Workload / axes:** backward-weight (`-F 4`) grouped convolution, depthwise (`cpg == kpg == 1`), fp16 and bf16, NHWC. 197 distinct shapes — ~half depthwise stages from 35 published architectures (mobilenet v1–v4, efficientnet b0–b6/v2, convnext/convnextv2, replknet/unireplknet, shufflenetv2, ghostnet, mnasnet, xception, yolo11/12, …), ~half a stratified random grid including a deliberate ~12% unmergeable minority. - **Baseline:** the configuration that shipped at this PR's merge-base (`2489f0b9153`) — same tile, `Gm = 1`. Ratios are against that, not against an oracle, because that is what the change is worth. - **Method:** `measure_degrees.py`, 2 warmup + 10 timed iterations per config, tile pinned, degree axis fully open, split-K open, epilogue pinned to two-stage so the comparison isolates the degree. Both arms measured in the same process on the same silicon. At these measured configurations, against the previously shipped point: | | ratio | |---|---| | geometric mean over 197 shapes | **1.98x** | | median | 1.25x | | p90 | 7.12x | | max | 36.7x | | min | 0.97x | 62/197 shapes select `Gm = 1` and are bit-for-bit the previous behaviour. 90/197 land within ±2%. One shape regresses beyond 2% (0.971x, `C128 K128 G128 5x5`). Against the best degree available at the shipped tile, the policy captures 0.998 of the oracle, and picks the exact best on 97 of the 135 shapes that have more than one admissible degree; of the other 38 the worst is 0.954x and none falls below 0.95x. **Honest losses:** the three rejected ladder policies all scored a higher geometric mean (up to 2.30x) by spending a wider `tile_n` — and all carried 20–45 regressions out of 197, worst 0.462x. Buying the mean with a long tail of halvings was not a trade worth shipping. Details and the rejected fits are in the case study. ## Correctness - **Numeric gate in the measured run:** `--verify` was forced for every config; 13,244 ranked configurations across three `tile_n` rungs, **13,244 PASS, 0 FAIL**. Checked against an independent torch reference at relative error `< 5e-2` for 16-bit compute (`benchmark_implicit_gemm_conv.py:288`). No unverified kernel was allowed to set an oracle or a ratio. - **On-GPU suite:** `library/tests/test_conv_wgrad_correctness.py` — 66 passed, 346 subtests. - **Selection suite:** 512 passed, 2279 subtests across `library/tests/dispatch/`, `test_conv_wgrad_group_merge_gate.py`, `test_library_layering.py`. `TestWgradMergeDegree::test_every_choice_is_admissible` is the load-bearing gate: at `split_k == -1` the kernel gate's split-K clause is vacuous, so a degree the gate refuses does not merely merge badly — `support()` rejects and depthwise wgrad stops dispatching entirely. That test replays every selected degree through the real `wgrad_group_merge_available`, so a clause that moves in the kernel and not in the selector fails loudly. The second commit pins a separate invariant: merged + `split_k > 1` must be two-stage. A merged tile computes a `Gm x Gm` block of group *pairs* and wants only the diagonal; the packed-atomic epilogue cannot mask off the rest, so taking it would accumulate an off-diagonal pair into a live `dW` element — a wrong gradient with nothing raised. ## Reproducing All of it, from `examples/gfx950/conv_wgrad/merge_degree_model/`: ```bash python3 gen_shapes.py --count 200 --model-cap 100 -o shapes_wgrad.txt python3 measure_degrees.py shapes_wgrad.txt -o deg_tn64.csv python3 score_policy.py --shipped deg_tn64.csv The corpus is checked in so results are reproducible; the sweep CSVs are not — they are raw per-kernel timings carrying absolute numbers. score_policy.py degrades cleanly when they are absent. Narrative and the rejected fits: wgrad_merged_groups_case_study.md. JIRA-ID : AICK-2486 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…grad on gfx950 (ROCm#12951) ## Summary Depthwise backward-weight convolution has had merged-group (`Gm`) support in the kernel since ROCm#12359, but nothing ever asked for it. `group_merge` defaults to 1 and `dispatch_conv_grouped` never set it, so the only ways to reach a merged kernel were to hand-construct a spec in a test or pass `--group-merge N` on the benchmark CLI. Every production depthwise wgrad ran unmerged. This PR adds the selection policy — `_wgrad_merge_degree()` — plus the plumbing that lets a non-default degree survive the trip from spec selection to launch. No kernel capability is added or changed. Why it matters: in wgrad the per-group GEMM is `kpg x (Z*Y*X*cpg) x (N*Ho*Wo)`, so depthwise (`cpg == kpg == 1`) makes the free axis one element wide and every B load scalar. Merging makes that run `Gm` elements long — `Gm` *is* the load vector width. ## What changed `library/dispatch/grouped_convolution.py`: - **`_wgrad_merge_degree()`** — the policy. Picks the largest degree the kernel's own gate admits at the tile that already ships. - **`ConvGroupedSpec.group_merge`** field, forwarded by `to_wgrad_spec()`, and emitted by `kernel_name()` as `gm{N}` — merging changes the GEMM, so merged and unmerged kernels must not collide on the compile-cache key. - **`_wgrad_grid()`** — z extent becomes `(groups / Gm) * split_k`; without this we would launch `Gm x` too many CTAs. - **`_resolve_wgrad_split_k()`** — split-K is chosen from GEMM dims that merging scales, and merged + `split_k > 1` is routed to two-stage. The workspace sizing is deliberately left **unmerged**, mirroring `wgrad_two_stage_workspace_nbytes`: merging changes how many CTAs write `dW`, not how much `dW` there is. Scaling it by `gm` would overstate the allocation quadratically and start refusing two-stage on problems that fit. ### There is no fitted constant, and that is the result The forward selector (ROCm#12793) is a fitted closed form with eight constants. Backward-weight ended up with none. Forward puts `Gm` on GemmN+GemmK, trading vector width against a working set growing in `Y*X*Gm` — a genuine interior optimum. Wgrad puts `Gm` on GemmM+GemmN and the gate caps it at `Z*Y*X*cpg*Gm <= tile_n`; with the tile fixed, the admissible set is two or three degrees wide and its top is nearly always best. Three richer policies were fitted and rejected; `score_policy.py` re-derives the rejection from the raw sweeps. ## Measured evidence Relative ratios only, per `platform/AGENTS.md` §Compliance. No absolute latency, throughput, or achieved-FLOP figure appears here or in any commit message. - **Hardware target:** gfx950. - **Workload / axes:** backward-weight (`-F 4`) grouped convolution, depthwise (`cpg == kpg == 1`), fp16 and bf16, NHWC. 197 distinct shapes — ~half depthwise stages from 35 published architectures (mobilenet v1–v4, efficientnet b0–b6/v2, convnext/convnextv2, replknet/unireplknet, shufflenetv2, ghostnet, mnasnet, xception, yolo11/12, …), ~half a stratified random grid including a deliberate ~12% unmergeable minority. - **Baseline:** the configuration that shipped at this PR's merge-base (`2489f0b9153`) — same tile, `Gm = 1`. Ratios are against that, not against an oracle, because that is what the change is worth. - **Method:** `measure_degrees.py`, 2 warmup + 10 timed iterations per config, tile pinned, degree axis fully open, split-K open, epilogue pinned to two-stage so the comparison isolates the degree. Both arms measured in the same process on the same silicon. At these measured configurations, against the previously shipped point: | | ratio | |---|---| | geometric mean over 197 shapes | **1.98x** | | median | 1.25x | | p90 | 7.12x | | max | 36.7x | | min | 0.97x | 62/197 shapes select `Gm = 1` and are bit-for-bit the previous behaviour. 90/197 land within ±2%. One shape regresses beyond 2% (0.971x, `C128 K128 G128 5x5`). Against the best degree available at the shipped tile, the policy captures 0.998 of the oracle, and picks the exact best on 97 of the 135 shapes that have more than one admissible degree; of the other 38 the worst is 0.954x and none falls below 0.95x. **Honest losses:** the three rejected ladder policies all scored a higher geometric mean (up to 2.30x) by spending a wider `tile_n` — and all carried 20–45 regressions out of 197, worst 0.462x. Buying the mean with a long tail of halvings was not a trade worth shipping. Details and the rejected fits are in the case study. ## Correctness - **Numeric gate in the measured run:** `--verify` was forced for every config; 13,244 ranked configurations across three `tile_n` rungs, **13,244 PASS, 0 FAIL**. Checked against an independent torch reference at relative error `< 5e-2` for 16-bit compute (`benchmark_implicit_gemm_conv.py:288`). No unverified kernel was allowed to set an oracle or a ratio. - **On-GPU suite:** `library/tests/test_conv_wgrad_correctness.py` — 66 passed, 346 subtests. - **Selection suite:** 512 passed, 2279 subtests across `library/tests/dispatch/`, `test_conv_wgrad_group_merge_gate.py`, `test_library_layering.py`. `TestWgradMergeDegree::test_every_choice_is_admissible` is the load-bearing gate: at `split_k == -1` the kernel gate's split-K clause is vacuous, so a degree the gate refuses does not merely merge badly — `support()` rejects and depthwise wgrad stops dispatching entirely. That test replays every selected degree through the real `wgrad_group_merge_available`, so a clause that moves in the kernel and not in the selector fails loudly. The second commit pins a separate invariant: merged + `split_k > 1` must be two-stage. A merged tile computes a `Gm x Gm` block of group *pairs* and wants only the diagonal; the packed-atomic epilogue cannot mask off the rest, so taking it would accumulate an off-diagonal pair into a live `dW` element — a wrong gradient with nothing raised. ## Reproducing All of it, from `examples/gfx950/conv_wgrad/merge_degree_model/`: ```bash python3 gen_shapes.py --count 200 --model-cap 100 -o shapes_wgrad.txt python3 measure_degrees.py shapes_wgrad.txt -o deg_tn64.csv python3 score_policy.py --shipped deg_tn64.csv The corpus is checked in so results are reproducible; the sweep CSVs are not — they are raw per-kernel timings carrying absolute numbers. score_policy.py degrades cleanly when they are absent. Narrative and the rejected fits: wgrad_merged_groups_case_study.md. JIRA-ID : AICK-2486 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

ISSUE ID: AICK-1945
Motivation
A depthwise group has cpg == kpg == 1, so both GEMM operands are single channels of an NHWC tensor: the free axis is one element wide, every global load degenerates to a scalar, and the (n, ho, wo) magic-division address decode is repeated per channel. Merging Gm adjacent groups into one workgroup makes the free axis a contiguous channel run, which is what the CK backend does with NumGroupsToMerge.
This adds the capability and makes the wgrad sweep evaluate it. It is not a win on every shape: merging divides the grid by Gm, so it trades parallelism for wider loads, and whether that pays depends on the group count. The sweep therefore measures both options rather than assuming one.
Technical Details
Removes three docstrings claiming is_valid_wgrad_spec rejects Gm>1, no such field existed.
Test Plan
Test Result
check_byte_identity.py GREEN 70/70 · wgrad correctness 72 OK (1 skip) · grouped wgrad dispatch 24 OK · split-K units 9 passed · layering clean. All Gm degrees match the reference with relative error identical to Gm=1, i.e. bf16 output rounding only.
On a g=512 depthwise shape the merged family is measurably slower than the split-K family and the sweep correctly ranks it below, merging divides the grid by Gm, and at split_k=1 nothing refills it. The value here is that both are now reachable and comparable; shapes with fewer groups, where groups/Gm still fills the device, are the ones expected to favour merging. Measured numbers are on the internal page, not here.
test_rocke_ci_static reports golden drift on 6 conv_wgrad/*_g4_spk4 entries. Pre-existing, reproduced on a clean checkout with this branch stashed; traces to 51fcea2. Not re-blessed here.
Submission Checklist