Skip to content

fix(ck_tile): report the D-tensor layout rejection instead of failing silently - #11701

Merged
CongMa13 merged 8 commits into
ROCm:developfrom
ThomasNing:users/ThomasNing/ck-loud-d-layout-rejection
Oct 9, 2026
Merged

CongMa13 merged 8 commits into
ROCm:developfrom
ThomasNing:users/ThomasNing/ck-loud-d-layout-rejection

Conversation

@ThomasNing

@ThomasNing ThomasNing commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

ISSUE ID : ROCm/composable_kernel#3766

Ported from ROCm/composable_kernel#3768 because that repository is now a read-only mirror. This PR applies the same change to the canonical projects/composablekernel tree.

Addresses the first ask in ROCm/composable_kernel#3766.

UniversalGemmKernel::IsSupportedArgument rejects any D tensor whose layout differs from CLayout:

if(std::is_same_v<DiLayout, CLayout> == false)
{
    DTensorIsValid = false;   // no message
}

It is the only rejection in that function that says nothing — every other one emits CK_TILE_ERROR under CK_TILE_LOGGING. The kernel simply never runs, and the reason has to be found by reading the source. We lost an afternoon to it.

The case that hits this is not exotic: a broadcast scale D — an M-vector with a column layout, paired with a row-major C — i.e. per-token scales expressed as a D tensor.

This PR only makes the rejection speak, matching the surrounding convention and naming the offending D index. The deeper asks in ROCm/composable_kernel#3766 (support column-layout D in the CShuffle epilogue's distribution, or document RowColQuant as the intended route) are unchanged and left open.

Compile-checked with hipcc -fsyntax-only --offload-arch=gfx950, with the unpatched header as a baseline.

🤖 Generated with Claude Code

… silently

UniversalGemmKernel::IsSupportedArgument rejects any D tensor whose layout
differs from CLayout, and is the only rejection in that function that says
nothing: every other one emits CK_TILE_ERROR under CK_TILE_LOGGING. The kernel
simply never runs, and the reason has to be found by reading the source -- we
lost an afternoon to it.

The case that hits this is not exotic: a broadcast scale D (an M-vector with a
column layout, paired with a row-major C), i.e. per-token scales expressed as a
D tensor.

Match the surrounding convention and name the offending D index.

Compile-checked with hipcc --offload-arch=gfx950 against the unpatched header
as a baseline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@therock-pr-bot

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

Copy link
Copy Markdown

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

@assistant-librarian assistant-librarian Bot added the external contribution Code contribution from users community.. label Sep 4, 2026
@ThomasNing

Copy link
Copy Markdown
Contributor Author

Addressed the unit-test policy feedback in 05ea757c98:

  • added a host-side regression test that enables CK_TILE_LOGGING, exercises the mismatched D-tensor layout rejection, and asserts that the diagnostic names D0;
  • wired the test into the CK Tile GEMM test target.

The policy table now reports Unit Test: Pass. The remaining pre-commit gate is awaiting maintainer approval because this is a fork PR: https://github.com/ROCm/rocm-libraries/actions/runs/33921367592

@illsilin

Copy link
Copy Markdown
Contributor

Hi Thomas, thanks for the pr. Unfortunately, the compilation for gfx11 fails. Please fix.

@ThomasNing

Copy link
Copy Markdown
Contributor Author

@illsilin Fixed the gfx11 compilation issue in bb9872f. The synthetic argument-validation fixture models the XDL pipeline interface, so its test target is now registered inside the existing gfx94/gfx95 block instead of the outer block that also builds gfx11/gfx12 WMMA tests. The production header change remains compiled through the real WMMA UniversalGemmKernel users on gfx11/gfx12.

@ThomasNing

Copy link
Copy Markdown
Contributor Author

@illsilin Fixed the gfx11 compilation blocker in bb9872f. The new synthetic argument-validation fixture models the XDL pipeline interface, but it was registered outside the existing architecture split and therefore compiled in the gfx11/gfx12 WMMA build. Its target is now scoped to the existing gfx94/gfx95 XDL block. The production header change remains compiled by the real WMMA kernel targets; only this synthetic fixture is excluded from WMMA builds.

@AviralGoelAMD

AviralGoelAMD commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Hey Thomas!!!!!!
The new test doesn’t build on gfx942 or gfx950. Its mock pipeline is missing the input data-type and layout definitions that UniversalGemmKernel expects: ADataType, BDataType, ALayout, and BLayout.

@AviralGoelAMD

Copy link
Copy Markdown
Contributor

The new error identifies which D tensor failed, but it does not show the actual D layout or the required C layout. Could we include DiLayout::name and CLayout::name in the message and verify both in the test?

@AviralGoelAMD AviralGoelAMD 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 one comment

@ThomasNing

Copy link
Copy Markdown
Contributor Author

Addressed both outstanding review comments in 4df7071:

  • completed the synthetic pipeline interface with ADataType, BDataType, ALayout, and BLayout, so it instantiates on gfx942/gfx950;
  • expanded the D-layout rejection diagnostic to show D{i}'s actual layout and C's required layout, and asserted the complete ColumnMajor/RowMajor wording in the regression test.

git diff --check passes. I could not run the GPU test locally because this checkout has no ROCm compiler/build tree; the supplied interactive cluster gateway did not complete a non-interactive SSH session.

@ThomasNing

Copy link
Copy Markdown
Contributor Author

@illsilin Could you take another look?

@illsilin

Copy link
Copy Markdown
Contributor

Compilation for gfx950 fails.

@ThomasNing

Copy link
Copy Markdown
Contributor Author

@illsilin Hi Illia,

Could you share the errors log in the CI? I cannot see our CI dashboard anymore.

…ow reads

UniversalGemmKernel::IsLargeTensorGlobalLoadSupported reads
BlockGemmShape::PermuteB and GemmPipeline::Preshuffle, neither of which
the mock types declared, so instantiating IsSupportedArgument failed to
compile. Add both, matching the defaults in TileGemmShape and
GemmPipelineProblem.

Mark kclusterM/N/K and BlockSize [[maybe_unused]]: they are declared to
mirror the real types but never read, which -Weverything -Werror rejects
for an anonymous-namespace type.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@CongMa13
CongMa13 enabled auto-merge (squash) October 3, 2026 21:28
@CongMa13
CongMa13 merged commit d4c2683 into ROCm:develop Oct 9, 2026
162 of 164 checks passed
assistant-librarian Bot pushed a commit to ROCm/composable_kernel that referenced this pull request Oct 9, 2026
fix(ck_tile): report the D-tensor layout rejection instead of
 failing silently (#11701)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ISSUE ID : #3766

> Ported from #3768 because that repository is now
a read-only mirror. This PR applies the same change to the canonical
`projects/composablekernel` tree.

Addresses the first ask in #3766.

`UniversalGemmKernel::IsSupportedArgument` rejects any D tensor whose
layout differs from `CLayout`:

```cpp
if(std::is_same_v<DiLayout, CLayout> == false)
{
    DTensorIsValid = false;   // no message
}
```

It is the **only** rejection in that function that says nothing — every
other one emits `CK_TILE_ERROR` under `CK_TILE_LOGGING`. The kernel
simply never runs, and the reason has to be found by reading the source.
We lost an afternoon to it.

The case that hits this is not exotic: a **broadcast scale D** — an
M-vector with a column layout, paired with a row-major C — i.e.
per-token scales expressed as a D tensor.

This PR only makes the rejection speak, matching the surrounding
convention and naming the offending D index. The deeper asks in
#3766 (support column-layout D in the CShuffle
epilogue's distribution, or document RowColQuant as the intended route)
are unchanged and left open.

Compile-checked with `hipcc -fsyntax-only --offload-arch=gfx950`, with
the unpatched header as a baseline.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@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

Labels

external contribution Code contribution from users community.. project: composablekernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants