-
Notifications
You must be signed in to change notification settings - Fork 140
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
[CK_TILE] Add GetName for GEMM kernels #1791
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.
I think it would be beneficial to make it much more comprehensive. Lets add also the size of warp tiles next to block tile. Additionally I'd add similar functions to Pipelines and block gemms, so that later on during benchmarking we could easily get report with what instance we're working on.
include/ck_tile/ops/gemm/pipeline/gemm_pipeline_ag_bg_cr_comp_v3.hpp
Outdated
Show resolved
Hide resolved
std::ostringstream oss; | ||
oss << s; | ||
return oss.str(); |
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.
Wouldn't this print just integer value?
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.
Minor thing left.
|
||
inline std::string GemmPipelineSchedulerToString(const ck_tile::GemmPipelineScheduler& s) | ||
{ | ||
std::ostringstream oss; |
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.
thread_local
std::ostringstream oss ? then an object of this storage class is created when the thread of execution passes through its definition and is destroyed immediately when execution leaves the lexical.
@@ -71,3 +72,10 @@ inline std::ostream& operator<<(std::ostream& os, const ck_tile::TailNumber& s) | |||
} | |||
return os; | |||
} | |||
|
|||
inline std::string GemmPipelineSchedulerToString(const ck_tile::GemmPipelineScheduler& s) |
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.
[[nodiscard]] fn?
// clang-format off | ||
using P_ = GemmPipeline; | ||
|
||
auto prec_str = [&] () { |
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.
align? - here and others prec_str fn
|
||
inline std::string GemmPipelineSchedulerToString(const ck_tile::GemmPipelineScheduler& s) | ||
{ | ||
std::ostringstream oss; |
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.
oss.str("")? - it could be fine to clean buffer before
|
||
return _SS_("gemm_problem_") + | ||
_TS_(VectorLoadSize) + "x" + _TS_(kBlockSize) + "_" + | ||
_TS_(kPadM) + "x" + _TS_(kPadN) + "x" + _TS_(kPadK) + "_" + |
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.
What do you think about the concat() function, to pass Scheduler and other params?
Proposed changes
Add GetName for GEMM kernels to easily print its name and a few other details.
Checklist
Please put an
x
into 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-format
on all changed filesDiscussion
Small change. I am also open to print other kernel parameters.