-
Notifications
You must be signed in to change notification settings - Fork 265
[TILE ENGINE] Restructure Tile Engine's benchmarking and profiling #3546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR restructures the Tile Engine's benchmarking and profiling infrastructure to reduce code duplication and improve maintainability by introducing a base class architecture for GEMM operations and consolidating common utilities.
Changes:
- Introduced base
GemmProfilerandGemmBenchmarkclasses with template-based inheritance for different GEMM variations - Consolidated common functionality (metrics, settings, performance results) into shared utility files
- Refactored Python benchmarking scripts to use inheritance and shared utility modules
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tile_engine/ops/gemm/gemm_universal/gemm_universal_profiler.hpp | New profiler class inheriting from base GemmProfiler template |
| tile_engine/ops/gemm/gemm_universal/gemm_universal_benchmark_single.cpp | Updated to use new profiler architecture and DataTypeTraits from ck_tile namespace |
| tile_engine/ops/gemm/gemm_universal/gemm_universal_benchmark.py | Refactored to inherit from shared GemmBenchmark base class |
| tile_engine/ops/gemm/gemm_universal/gemm_universal_benchmark.hpp | New header containing gemm_host_reference function moved from deleted file |
| tile_engine/ops/gemm/gemm_profiler.hpp | New base profiler template class with common benchmarking logic |
| tile_engine/ops/gemm/gemm_benchmark.hpp | New base header with GemmProblem struct and comparison utilities |
| tile_engine/ops/gemm/gemm_benchmark.py | New base Python class for GEMM benchmarking with shared functionality |
| tile_engine/ops/common/utils.hpp | New common utilities header with shared structs and helper functions |
| tile_engine/ops/common/benchmark_utils.py | New Python utilities module with shared benchmarking functions |
| tile_engine/ops/gemm/gemm_preshuffle/* | Updated to use base classes and remove duplicated code |
| tile_engine/ops/gemm/gemm_multi_d/* | Updated to use base classes and restructured problem definition |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tile_engine/ops/gemm/gemm_multi_d/gemm_multi_d_benchmark_single.cpp
Outdated
Show resolved
Hide resolved
tile_engine/ops/gemm/gemm_multi_d/gemm_multi_d_benchmark_single.cpp
Outdated
Show resolved
Hide resolved
tile_engine/ops/gemm/gemm_preshuffle/gemm_preshuffle_common.hpp
Outdated
Show resolved
Hide resolved
|
I'm noting here that #3434 seems to have broken the test target generation in https://github.com/ROCm/composable_kernel/blob/develop/test/ck_tile/gemm_tile_engine/CMakeLists.txt, as the base |
This change restructures the Benchmark structs into 3 files. There is an addition of a base class for all GEMM benchmarks, derived classes for Universal GEMM, multi dim GEMM, and GEMM preshuffle. Common functions have been relocated into a common directory. For any derived base classes, only the redefination of the constructor is needed, significantly mitigating the need for duplicated code.
This change restructures the profiling process in Tile Engine into a base class for the Profiling and Problem structs. With this all files needed for Tile Engine will have a base struct and files in the gemm/ directory that can be extended for each GEMM variant. Only the Problem and Profiler structs along with the reference functions need to be defined. Profiling functions that are common to each operation have been moved into a common utility file.
9662629 to
239ebe0
Compare
I will be fixing that script in #3514 to run the newly named script based on the restructure from #3434. gemm_streamk is not part of the Tile Engine restructure in these PRs. When we add gemm_streamk to the restructured format, we will update the corresponding tests to use the correct script. |
Proposed changes
This PR introduces a restructure for the benchmarking and profiling aspects of CK Tile's Tile Engine, expanding on the groundwork from this previous PR and outlined in this design document. The files have been restructured to have a base structure for benchmarks, profiling and problem descriptions that is inherited from for each variation of GEMM. All common functions across the benchmarking and profiling files have been moved into newly added common utility files. This should greatly reduce the duplicated code present in Tile Engine and make it simpler to add in new operations.
Checklist
Please put an
xinto the boxes that apply. You can also fill these out after creating the PR. If you're not sure, please don't hesitate to ask.clang-formaton all changed files