Support for new performance tool suite : LIKWID #6053
Conversation
srinivasyadav18
requested review from
aurianer,
msimberg,
biddisco and
hkaiser
as code owners
| # file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) | ||
|
|
||
| # enable likwid module only if HPX_WITH_LIKWID is set | ||
| if(NOT ${HPX_WITH_LIKWID}) |
There was a problem hiding this comment.
| if(NOT ${HPX_WITH_LIKWID}) | |
| if(NOT HPX_WITH_LIKWID) |
| hpx_execution | ||
| hpx_runtime_local | ||
| CMAKE_SUBDIRS examples tests | ||
| ) |
There was a problem hiding this comment.
I'm almost certain our cmake-format check will complain about this.
| likwid | ||
| ====== | ||
|
|
||
| TODO: High-level description of the module. |
There was a problem hiding this comment.
Would you mind editing this and add a short description of what the module is doing?
| on_start_ = [name]() {likwid_markerStartRegion(name.data());}; | ||
| on_stop_ = [name]() {likwid_markerStopRegion(name.data());}; |
There was a problem hiding this comment.
Do we have to copy the name twice?
There was a problem hiding this comment.
Also, shouldn't this use name.c_str() instead (i.e. ensure the sequence of bytes is zero-terminated)?
| thread_hook on_start_; | ||
| thread_hook on_stop_; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Would you mind adding a CTAD deduction guide that would possibly simplify code? That might even make obsolete the make_likwid_executor helper below.
There was a problem hiding this comment.
Actually, things should work with out a deduction guide (and in fact, your test proves that). Let's just get rid of the make_likwid_executor function below.
| #pragma once | ||
|
|
||
| #include <likwid.h> | ||
| #include <hpx/modules/runtime_local.hpp> |
There was a problem hiding this comment.
Please #include the HPX headers first. Also, shouldn't we wrap all of this header's content in a #if defined(HPX_HAVE_LIKWID)/#endif?
| void likwid_thread_init() | ||
| { | ||
| auto prev = hpx::get_thread_on_start_func(); | ||
| hpx::register_thread_on_start_func | ||
| ( | ||
| [prev](std::size_t local_thread_num,std::size_t global_thread_num, | ||
| char const* pool_name, char const* name_postfix) | ||
| { | ||
| likwid_markerThreadInit(); | ||
| if (!prev.empty()) | ||
| prev(local_thread_num, global_thread_num, pool_name, name_postfix); | ||
| } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Can we move this implementation into the cpp file?
| "HPX main exited with non-zero status"); | ||
|
|
||
| return hpx::util::report_errors(); | ||
| } |
There was a problem hiding this comment.
Is there a way for this test to actually verify that Likwid is being used?
Proposed Changes
Adds support to use LIKWID using executors
likwid_executor