Skip to content

feat(rocke): split-C and grouped LDS-halo direct conv fwd kernels - #13993

Merged
bartekxk merged 3 commits into
developfrom
users/barkocot/fwd-improvements
Oct 21, 2026
Merged

bartekxk merged 3 commits into
developfrom
users/barkocot/fwd-improvements

Conversation

@bartekxk

Copy link
Copy Markdown
Contributor

Forward direct conv on gfx950 lost to MIOpen on small maps with deep channels (too few tiles to fill the CUs) and on grouped convs with small channels per group (an MFMA tile mostly idle). Two new variants of the non-grouped LDS-halo kernel address both:

  • split_c: the channel loop is split across workgroups that write fp32 partial sums to a workspace, folded into y by a second-pass reduction kernel (direct_split_reduce, cached under direct_fwd_helper).
  • grouped / group_merge (AOT variant direct_grouped_halo): runtime groups and channels, small groups merged into one MFMA problem through block-diagonal weights.

Supporting changes:

  • Batches whose activations exceed the kernels' 32-bit buffer range run as balanced per-chunk launches over views of the same buffers.
  • The torch reference convolves large batches in chunks: the backend returned garbage once an fp32 operand passed 2**31 bytes.
  • After --compile-all, grouped-halo and split-C entries whose binary spills, or whose waves_per_eu=3 code equals the waves_per_eu=None twin's, are dropped from the cache (a pure-Python ELF read of .text and the kernel descriptors); the dense unsplit grid is left untouched.

Python and C++ engines emit byte-identical IR (llvm20 and llvm22 GREEN).

JIRA ID: AICK-2328

Forward direct conv on gfx950 lost to MIOpen on small maps with deep
channels (too few tiles to fill the CUs) and on grouped convs with small
channels per group (an MFMA tile mostly idle). Two new variants of the
non-grouped LDS-halo kernel address both:

- split_c: the channel loop is split across workgroups that write fp32
  partial sums to a workspace, folded into y by a second-pass reduction
  kernel (direct_split_reduce, cached under direct_fwd_helper).
- grouped / group_merge (AOT variant direct_grouped_halo): runtime groups
  and channels, small groups merged into one MFMA problem through
  block-diagonal weights.

Supporting changes:
- Batches whose activations exceed the kernels' 32-bit buffer range run
  as balanced per-chunk launches over views of the same buffers.
- The torch reference convolves large batches in chunks: the backend
  returned garbage once an fp32 operand passed 2**31 bytes.
- After --compile-all, grouped-halo and split-C entries whose binary
  spills, or whose waves_per_eu=3 code equals the waves_per_eu=None
  twin's, are dropped from the cache (a pure-Python ELF read of .text and
  the kernel descriptors); the dense unsplit grid is left untouched.

Python and C++ engines emit byte-identical IR (llvm20 and llvm22 GREEN).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@bartekxk bartekxk self-assigned this Oct 20, 2026
@bartekxk
bartekxk requested review from a team as code owners October 20, 2026 12:55
@therock-pr-bot

therock-pr-bot Bot commented Oct 20, 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 20, 2026 •

Copy link
Copy Markdown

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

Copilot AI 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.

🟡 Changes recommended

Input validation and reference byte-limit handling remain incomplete, and pruning retains unnecessary compiled-code data.

4 open issues
What changed in this PR

Adds split-C and grouped LDS-halo forward convolution variants to improve rocKE utilization on small maps and grouped workloads.

Changes:

  • Implements matching Python/C++ kernels and split-C reduction.
  • Adds batch chunking, cache integration, and compiled-binary pruning.
  • Extends correctness, cache, and IR parity coverage.

Paths below are relative to dnn-providers/hip-kernel-provider/rocke/.

File Description
platform/​tests/​instances/​differential/​golden/​llvm_gfx_all.json Adds IR golden hashes.
platform/​python/​rocke/​core/​backend.py Serializes new spec fields.
platform/​cpp/​instances/​common/​conv_direct_nongrouped.cpp Implements C++ kernel variants and reduction.
platform/​cpp/​include/​rocke/​instance_conv_direct_nongrouped.h Exposes new specs and APIs.
platform/​cpp/​bindings/​rocke_engine.cpp Reads new tuning fields.
library/​tests/​test_direct_kernel_cache.py Tests planning, chunking, and pruning.
library/​tests/​test_direct_conv_correctness.py Adds GPU correctness cases.
library/​tests/​test_builder_signature_contract.py Registers the reduction spec.
library/​tests/​parity/​conv_direct_grouped_emit.py Adds Python parity cases.
library/​tests/​parity/​conv_direct_grouped_emit.c Adds matching C++ parity cases.
library/​kernels/​common/​conv_direct_nongrouped.py Implements Python variants and tuning grids.
library/​builders/​common/​conv_reference.py Chunks large reference batches.
library/​benchmarks/​common/​kernel_cache.py Supports helper caching and entry removal.
library/​benchmarks/​common/​direct_kernel_sweep.py Integrates variants, launch plans, and pruning.
library/​benchmarks/​common/​benchmark_direct_conv.py Includes forward helpers in cache dispatch.

Review effort: Balanced

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

split_c seams like a great idea

bartekxk and others added 2 commits October 20, 2026 16:36
…rnels

- Retain SHA-256 digest instead of compiled bytes in _elf_code_facts;
  prevents the cache from holding megabytes of ELF text per sweep entry.
- Reject images that exceed the fp32 2**31 byte budget before the batch
  split in conv_reference rather than silently clamping per_chunk to 1.
- Reject grouped=True with groups < 1 in both the Python and C++ emitters
  (byte-identical rejection path); add parity config 113 (groups=0) and
  a must-reject unit-test for groups in {0, 1, 2, -4}.
- Forward spec.split_c / spec.grouped / spec.group_merge directly in
  backend.py instead of using getattr defaults that would silently mask
  a missing attribute.

Byte-identity gate: GREEN (llvm20 + llvm22) for conv_direct_grouped.

Co-Authored-By: Claude <noreply@anthropic.com>
…arkocot/fwd-improvements

# Conflicts:
#	dnn-providers/hip-kernel-provider/rocke/library/benchmarks/common/direct_kernel_sweep.py
@bartekxk
bartekxk enabled auto-merge (squash) October 20, 2026 22:24

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

No new issues found. I agree with the following existing comments:

  • Retain code digests instead of all compiled bytes (copilot-pull-request-reviewer, #13993 (comment))
  • Reject images that exceed the fp32 byte budget (copilot-pull-request-reviewer, #13993 (comment))
  • Reject zero or negative group counts (copilot-pull-request-reviewer, #13993 (comment))
  • Reject missing configuration fields instead of applying defaults (copilot-pull-request-reviewer, #13993 (comment))

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

No new P0 or P1 issues were identified during this review. The changes are approved.

@bartekxk
bartekxk merged commit 3440eec into develop Oct 21, 2026
100 checks passed
@bartekxk
bartekxk deleted the users/barkocot/fwd-improvements branch October 21, 2026 01:45
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

cgmillette added a commit that referenced this pull request Oct 26, 2026
The differential golden had gone stale, because nothing ran the check:
675 of its 1116 LLVM IR entries (compared at llvm20) and 608 of its 1089
canonical IR entries no longer match the tree. Recorded afresh in the new
format, with the Python reference emitters, from a tree whose two-engine
gate is green at llvm20, llvm22 and llvm23:

- ll at llvm20 and llvm22: 82 families, 1474 configs per flavor. At
  llvm23: 83 families, 1578 configs, where gfx1250_lowering has configs
  that need LLVM 23 and are refused below it, and every
  conv_direct_dgrad_wmma config needs LLVM 23.
- ir_canonical: 82 families, 1561 configs (conv_abi's emitter has no IR
  mode). Canonical IR was checked to be identical at llvm20 and llvm23.

What the re-bless absorbs: every mismatch that could be traced is an
intended code generation change that did not re-bless this file -- the
shared LDS pool (#8844, about 425 configs), the ds.swizzle declaration
(#9583, about 177), renamed values in the attention transpose reads (#9133,
57), the smaller LDS pool (#10112, about 39), the flat work group size of
one-wave wave32 kernels (#12710, about 39), #9220 (6) and the conv changes
of #13109 (6). 441 entries were already stale at the start of the history
available here and cannot be attributed; no unexplained change was found
after it. The two repeated keys (conv_direct_grouped,
conv_wgrad_workspace_reduce), which hid their newer copies, are gone.

It also takes in the code generation changes merged since then, each
traced by recording the file at the commit and at its parent; the count is
per section unless a flavor is named:

- the magic division sum emitted without signed-overflow poison (#13802):
  215 configs in 19 families -- attention_unified 6,
  attention_unified_stress 32, conv_direct_grouped 16, conv_implicit_gemm
  26, conv_implicit_gemm_dgrad 31, conv_implicit_gemm_wgrad 42,
  deep_fused_conv_pool 2, gfx1201_deep_fused_conv_pool 5,
  gfx950_deep_fused_conv_pool 5, dynamic_helpers 2, fmha_splitkv_decode 6,
  img2col 5, moe_fused_mega 5, moe_gemm_fused 6, moe_sorting 6, pooling 4,
  sage_attention 4, sparse_attention 6, transpose 6;
- attention decoding on gfx942 and gfx950 (#13492): 23 changed and 25 new
  configs in attention_strided_kv, gfx942_attention_tiled_3d and
  gfx950_attention_tiled_3d;
- the f64 follow-ups (#13887): 3 changed and 1 new elementwise config, 5
  changed reduce configs;
- MX scale and fp6 decoding (#13885): 6 mx_gemm configs;
- the gfx950 conv dgrad speed-up (#13075): 36 new conv_direct_dgrad_ext
  configs; 22 changed, 15 new and 2 removed conv_implicit_gemm_dgrad
  configs; 2 changed and 3 new conv_direct_grouped configs; 2 new
  dynamic_helpers and 3 new target_intrinsics configs;
- removing the ds.read.tr16.b128 declaration (#14054): gfx1250_lowering,
  100 changed and 2 removed configs at llvm23 and in canonical IR, 5
  changed, 2 new and 4 removed at llvm20 and llvm22;
- optional LSE output on unified split-KV attention (#13955): 18
  gfx942_attention_tiled_3d and 16 gfx950_attention_tiled_3d configs per
  LLVM flavor, canonical IR unchanged;
- the running softmax max in 3D split-KV (#14276): 14
  attention_strided_kv, 18 gfx942_attention_tiled_3d and 16
  gfx950_attention_tiled_3d configs;
- depthwise 7x7 and grouped wgrad on batched MFMA (#14157): 7 changed, 10
  new and 1 removed conv_direct_grouped configs;
- the retirement of wmma_fmha_fwd (#13897): the gfx1151_wmma_fmha_fwd (157
  configs) and gfx1201_wmma_fmha_fwd (12) families are gone;
- new configs, every existing hash unchanged: the barrier_fence family
  (#13575, 10), one conv_implicit_gemm_dgrad config (#13782), 14
  conv_direct_grouped configs for the split-C and grouped LDS-halo kernels
  (#13993), one config each in fmha_splitkv_decode, sage_attention and
  sparse_attention (#13876), 23 conv_direct_grouped configs for the direct
  wgrad kernels (#13990), 2 conv_implicit_gemm_dgrad configs for
  patch-embedding dgrad (#13988), 6 f64_mma configs (#13794), 5
  conv_direct_grouped and 7 conv_implicit_gemm configs for depthwise and
  grouped conv forward (#13987), and the conv_direct_dgrad_wmma family (7
  configs, llvm23 and canonical IR only, #14156).

#13885, #13955 and #14276 updated the old file's entries for their
changes; the new llvm20 and canonical IR sections reproduce every one of
those hashes.

Co-Authored-By: Claude <noreply@anthropic.com>
cgmillette added a commit that referenced this pull request Oct 26, 2026
The differential golden had gone stale, because nothing ran the check:
675 of its 1116 LLVM IR entries (compared at llvm20) and 608 of its 1089
canonical IR entries no longer match the tree. Recorded afresh in the new
format, with the Python reference emitters, from a tree whose two-engine
gate is green at llvm20, llvm22 and llvm23:

- ll at llvm20 and llvm22: 82 families, 1474 configs per flavor. At
  llvm23: 83 families, 1578 configs, where gfx1250_lowering has configs
  that need LLVM 23 and are refused below it, and every
  conv_direct_dgrad_wmma config needs LLVM 23.
- ir_canonical: 82 families, 1561 configs (conv_abi's emitter has no IR
  mode). Canonical IR was checked to be identical at llvm20 and llvm23.

What the re-bless absorbs: every mismatch that could be traced is an
intended code generation change that did not re-bless this file -- the
shared LDS pool (#8844, about 425 configs), the ds.swizzle declaration
(#9583, about 177), renamed values in the attention transpose reads (#9133,
57), the smaller LDS pool (#10112, about 39), the flat work group size of
one-wave wave32 kernels (#12710, about 39), #9220 (6) and the conv changes
of #13109 (6). 441 entries were already stale at the start of the history
available here and cannot be attributed; no unexplained change was found
after it. The two repeated keys (conv_direct_grouped,
conv_wgrad_workspace_reduce), which hid their newer copies, are gone.

It also takes in the code generation changes merged since then, each
traced by recording the file at the commit and at its parent; the count is
per section unless a flavor is named:

- the magic division sum emitted without signed-overflow poison (#13802):
  215 configs in 19 families -- attention_unified 6,
  attention_unified_stress 32, conv_direct_grouped 16, conv_implicit_gemm
  26, conv_implicit_gemm_dgrad 31, conv_implicit_gemm_wgrad 42,
  deep_fused_conv_pool 2, gfx1201_deep_fused_conv_pool 5,
  gfx950_deep_fused_conv_pool 5, dynamic_helpers 2, fmha_splitkv_decode 6,
  img2col 5, moe_fused_mega 5, moe_gemm_fused 6, moe_sorting 6, pooling 4,
  sage_attention 4, sparse_attention 6, transpose 6;
- attention decoding on gfx942 and gfx950 (#13492): 23 changed and 25 new
  configs in attention_strided_kv, gfx942_attention_tiled_3d and
  gfx950_attention_tiled_3d;
- the f64 follow-ups (#13887): 3 changed and 1 new elementwise config, 5
  changed reduce configs;
- MX scale and fp6 decoding (#13885): 6 mx_gemm configs;
- the gfx950 conv dgrad speed-up (#13075): 36 new conv_direct_dgrad_ext
  configs; 22 changed, 15 new and 2 removed conv_implicit_gemm_dgrad
  configs; 2 changed and 3 new conv_direct_grouped configs; 2 new
  dynamic_helpers and 3 new target_intrinsics configs;
- removing the ds.read.tr16.b128 declaration (#14054): gfx1250_lowering,
  100 changed and 2 removed configs at llvm23 and in canonical IR, 5
  changed, 2 new and 4 removed at llvm20 and llvm22;
- optional LSE output on unified split-KV attention (#13955): 18
  gfx942_attention_tiled_3d and 16 gfx950_attention_tiled_3d configs per
  LLVM flavor, canonical IR unchanged;
- the running softmax max in 3D split-KV (#14276): 14
  attention_strided_kv, 18 gfx942_attention_tiled_3d and 16
  gfx950_attention_tiled_3d configs;
- depthwise 7x7 and grouped wgrad on batched MFMA (#14157): 7 changed, 10
  new and 1 removed conv_direct_grouped configs;
- the retirement of wmma_fmha_fwd (#13897): the gfx1151_wmma_fmha_fwd (157
  configs) and gfx1201_wmma_fmha_fwd (12) families are gone;
- new configs, every existing hash unchanged: the barrier_fence family
  (#13575, 10), one conv_implicit_gemm_dgrad config (#13782), 14
  conv_direct_grouped configs for the split-C and grouped LDS-halo kernels
  (#13993), one config each in fmha_splitkv_decode, sage_attention and
  sparse_attention (#13876), 23 conv_direct_grouped configs for the direct
  wgrad kernels (#13990), 2 conv_implicit_gemm_dgrad configs for
  patch-embedding dgrad (#13988), 6 f64_mma configs (#13794), 5
  conv_direct_grouped and 7 conv_implicit_gemm configs for depthwise and
  grouped conv forward (#13987), and the conv_direct_dgrad_wmma family (7
  configs, llvm23 and canonical IR only, #14156).

#13885, #13955 and #14276 updated the old file's entries for their
changes; the new llvm20 and canonical IR sections reproduce every one of
those hashes.

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.

4 participants