[CK_BUILDER]ckb add remining fwd conv device ops #3155
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ts and device operations.
…ect load template parameter in CK.
…ralize-conv-factory
- Added 5 DL descriptor structs (39 configurable parameters) - Added 10 C++20 concepts for type-safe validation - Updated factory to read all parameters from descriptors - Updated test helper to populate all descriptors - All tests passing (13/13 including 3 new DL tests)
…huffle_Large_Tensor - Add factory specialization for Large_Tensor device operation (conv_factory.hpp lines 1145-1265) - Add macro collision workaround using pragma push/pop (conv_factory.hpp lines 43-51) - Add test helper function run_test_DeviceGroupedConvFwdMultipleD_Xdl_CShuffle_Large_Tensor - Add builder test file test_ckb_conv_fwd_2d_large_tensor_fp16.cpp with 2 test cases - Update CMakeLists.txt to include new test file - Reuse existing ConvAlgorithm_DeviceGroupedConvFwdMultipleABD_Xdl_CShuffle descriptor - Map all 42 template parameters identical to regular XDL CShuffle - All 15 builder tests passing including 2 new Large_Tensor tests Completes Task 350: All 4 forward convolution device operations now supported in CK Builder.
JH-Leon-KIM-AMD
requested review from
ThomasNing,
afagaj,
andriy-ca,
aosewski,
asleepzzz,
bartekxk,
carlushuang,
cgmillette,
coderfeli,
geyyer,
illsilin,
poyenc,
qianfengz,
shumway,
tenpercent and
vidyasagar-amd
as code owners
vpietila-amd
previously approved these changes
Nov 5, 2025
Contributor
vpietila-amd
left a comment
vpietila-amd
left a comment
There was a problem hiding this comment.
Looks good to me. There's a PR #3154 that will reduce the code duplication in the test code. If that PR goes in first, we need to do the same changes for this PR too. But they are relatively straightforward changes. Once this PR is merged in the develop branch, I'll do another refactoring to remove the explicit device op flag from the signature.
| @@ -0,0 +1,69 @@ | |||
| // SPDX-License-Identifier: MIT | |||
| // Copyright (c) 2025, Advanced Micro Devices, Inc. All rights reserved. | |||
Contributor
There was a problem hiding this comment.
Check that the Copyright follows the convention introduced in PR #3150. Same for the other new test file.
JH-Leon-KIM-AMD
changed the title
- Change copyright format to: Copyright (C) Advanced Micro Devices, Inc., or its affiliates. - Reorder headers: Copyright first, then SPDX-License-Identifier - Updated files: * experimental/builder/test/conv/test_ckb_conv_fwd_2d_dl_fp16.cpp * experimental/builder/test/conv/test_ckb_conv_fwd_2d_large_tensor_fp16.cpp * experimental/builder/include/ck_tile/builder/device_op_types.hpp
shumway
approved these changes
Nov 7, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proposed changes
This PR adds factory support for the remaining forward convolution device operations in CK Builder.
Jira Ticket: https://amd-hub.atlassian.net/jira/software/c/projects/ALMIOPEN/boards/319/backlog?selectedIssue=ALMIOPEN-350
Task 350 - Add remaining forward convolution device operations:
DeviceGroupedConvFwdDlMultipleD_NHWC_KYXC_NHWK
DeviceGroupedConvFwdMultipleD_Xdl_CShuffle_Large_Tensor
This completes Task 350 - all 4 forward convolution device operations are now supported in CK Builder:
Checklist
clang-formaton all changed filesDiscussion
Design Decisions
1. DL Algorithm Descriptor (30 parameters)
ConvAlgorithm_DeviceGroupedConvFwdDlMultipleD_NHWC_KYXC_NHWKDlThreadConfigDescriptor,DlThreadClusterDescriptor,DlBlockTransferK0M0M1K1Descriptor, etc.2. Large_Tensor Descriptor Reuse (42 parameters)
SplitN=trueflag in device operation (not exposed in factory interface)ConvAlgorithm_DeviceGroupedConvFwdMultipleABD_Xdl_CShuffle3. Macro Collision Workaround
device_grouped_conv_fwd_multiple_abd_xdl_cshuffle.hpp(line 41) anddevice_grouped_conv_fwd_multiple_d_xdl_large_tensor_cshuffle.hpp(line 51) defineGridwiseGemmTemplateParametersmacro without#undef#pragma push_macro/#pragma pop_macroto isolate Large_Tensor header's macro scope