-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add tracer annotations to HIP code in ReSolve #134
Changes from all commits
f511c16
b25c1a0
8537b5e
d9d35da
9140c5a
918a794
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,10 +135,26 @@ target_include_directories(ReSolve INTERFACE | |
$<INSTALL_INTERFACE:include> | ||
) | ||
|
||
# TODO: Make this PRIVATE dependency (requires refactoring ReSolve code) | ||
target_link_libraries(ReSolve PUBLIC ${ReSolve_Targets_List}) | ||
target_link_libraries(ReSolve PRIVATE resolve_version) | ||
|
||
if(RESOLVE_USE_PROFILING) | ||
if(RESOLVE_USE_HIP) | ||
# Roctracer does not provide CMake target, so we use this hack here. | ||
# The assumption is roctracer lib and headers are installed at the same | ||
# place as the rest of ROCm. | ||
target_link_libraries(ReSolve PUBLIC "-lroctracer64 -lroctx64") | ||
elseif(RESOLVE_USE_CUDA) | ||
# Nothing to do for CUDA profiling for now. | ||
message(NOTICE "Profiling support enabled, but Re::Solve does not create tracer annotations for CUDA.") | ||
message(NOTICE "This profiling support option will have no effect.") | ||
else() | ||
# Noting to do for profiling on the host for now. | ||
message(NOTICE "Profiling support enabled, but Re::Solve does not create tracer annotations for host code.") | ||
message(NOTICE "This profiling support option will have no effect.") | ||
Comment on lines
+153
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could support this with a different profiling tool like caliper/tau... At the same time, I don't see why we couldn't use ROCm/CUDA profilers to view host code performance as well, albeit that would require a hack + messy CMake There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You are right, we can, it is just that I didn't do it in this PR. Probably the message should say "tracer annotation not implemented for ...". |
||
endif() | ||
endif(RESOLVE_USE_PROFILING) | ||
|
||
# Install targets | ||
install(TARGETS ReSolve | ||
EXPORT ReSolveTargets | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
#pragma once | ||
|
||
#ifdef RESOLVE_USE_PROFILING | ||
|
||
#ifdef RESOLVE_USE_HIP | ||
#include <roctracer/roctx.h> | ||
#define RESOLVE_RANGE_PUSH(x) roctxRangePush(x) | ||
#define RESOLVE_RANGE_POP(x) roctxRangePop(); \ | ||
roctxMarkA(x) | ||
#endif | ||
|
||
#else | ||
|
||
// Not using profiling | ||
#define RESOLVE_RANGE_PUSH(x) | ||
#define RESOLVE_RANGE_POP(x) | ||
|
||
#endif // RESOLVE_USE_PROFILING |
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'd suggest throwing a warning or error here to notify the user that profiling is currently only support with roctx.
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 believe 918a794 addresses this comment.
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.
cc #63