Skip to content

feat: support dimension sizes less than 8 - #11776

Merged
illsilin merged 33 commits into
developfrom
users/bidlekm/ck/lower-dim-gemm-enable
Sep 22, 2026
Merged

illsilin merged 33 commits into
developfrom
users/bidlekm/ck/lower-dim-gemm-enable

Conversation

@bidlekm

@bidlekm bidlekm commented Sep 8, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

When the inner inner matrix dimensin is not aligned with the vector loading size, the shape is rejected by IsSupportedArgument. To run these shapes, a switch to scalar loading is necessary in these cases.

This PR builds upon: feat: Compatible K when not a multiple of 8

Technical Details

There are two GEMM families affected: universal and batched.

Universal GEMM: gemm_utils.hpp already has a shared GemmConfigBase which every config inherits from, with kPadM/N/K, FixedVectorSize, VectorSizeA/B/C all defaulting to false/1. A new config got added which inherits from whichever GemmConfig is passed as a template parameter and overrides just the mentioned fields. run_gemm_example_with_layouts contains a decision tree: it computes what configuration to pick based on the vector load size and the passed shape.

Batched GEMM: batched_gemm.hpp configs are standalone structs with no shared base class, they never had kPadM/N/K/FixedVectorSize/VectorSizeA/B/C fields at all. A new config is added that has these values, but to let batched_gemm() run with either the pre-existing configs (which lack them) or the new one, an SFINAE wrapper with field detection (GemmConfigVectorSizeDefaults) was added. This resolves to the config itself if the field is present, or to an all-false/1 defaults struct otherwise, with the same overriding mechanism as in Universal GEMM.

Closes: #11776

Test Plan

Test Result

Submission Checklist

@bidlekm bidlekm self-assigned this Sep 8, 2026
@bidlekm
bidlekm requested a review from a team as a code owner September 8, 2026 09:42
@therock-pr-bot

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

Copy link
Copy Markdown

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

…s tile-size misaligned but vector-aligned

Adds needsPadM/N/K checks (independent of the existing vector-size
needsScalarAB/needsScalarC checks) to both the universal and batched
GEMM example dispatch, and a matching GemmConfigFixedVectorSize
fallback branch using natural (non-scalar) vector sizes with
kPadM/N/K forced on. Also adds test_gemm_unaligned_mnk.cpp covering
unaligned M/N (in addition to the existing unaligned-K coverage).

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

I have got questions on the compatibility of some pipelines, and whether we should ship this work as-is or whether we should look into it a bit further before merging.

@chris-tsiaousis-hpc chris-tsiaousis-hpc 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.

Left some comments and a more unified design proposal...

@bidlekm
bidlekm marked this pull request as ready for review September 17, 2026 08:29
@illsilin
illsilin enabled auto-merge (squash) September 18, 2026 20:00
@illsilin
illsilin merged commit 55e287a into develop Sep 22, 2026
102 checks passed
@illsilin
illsilin deleted the users/bidlekm/ck/lower-dim-gemm-enable branch September 22, 2026 15:57
assistant-librarian Bot pushed a commit to ROCm/composable_kernel that referenced this pull request Sep 22, 2026
feat: support dimension sizes less than 8
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

## Motivation

When the inner inner matrix dimensin is not aligned with the vector
loading size, the shape is rejected by `IsSupportedArgument`. To run
these shapes, a switch to scalar loading is necessary in these cases.

This PR builds upon: [feat: Compatible K when not a multiple of
8](ROCm/rocm-libraries#11537 (comment))
## Technical Details

There are two GEMM families affected: `universal` and `batched`.

`Universal` GEMM: `gemm_utils.hpp` already has a shared `GemmConfigBase`
which every config inherits from, with `kPadM/N/K, FixedVectorSize,
VectorSizeA/B/C` all defaulting to `false/1`. A new config got added
which inherits from whichever `GemmConfig` is passed as a template
parameter and overrides just the mentioned fields.
`run_gemm_example_with_layouts` contains a decision tree: it computes
what configuration to pick based on the vector load size and the passed
shape.

`Batched` GEMM: `batched_gemm.hpp` configs are standalone structs with
no shared base class, they never had
`kPadM/N/K/FixedVectorSize/VectorSizeA/B/C` fields at all. A new config
is added that has these values, but to let `batched_gemm()` run with
either the pre-existing configs (which lack them) or the new one, an
SFINAE wrapper with field detection (GemmConfigVectorSizeDefaults) was
added. This resolves to the config itself if the field is present, or to
an all-`false/1` defaults struct otherwise, with the same overriding
mechanism as in `Universal` GEMM.

Closes: #11776

## Test Plan

<!-- Explain any relevant testing done to verify this PR. -->

## Test Result

<!-- Briefly summarize test outcomes. -->

## Submission Checklist

- [ ] Look over the contributing guidelines at
https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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