From 340c7cb5536e241f364038e2890326a0d20f6883 Mon Sep 17 00:00:00 2001 From: Laurent Morichetti Date: Tue, 30 Aug 2022 18:47:00 -0700 Subject: [PATCH] SWDEV-351980 - Use the new hipRegister/RemoveAsyncActivityCallback Remove the hipInitActivityCallback and use the new hipRegister/ RemoveActivityCallback which allows distinct memory pools to be used for HIP_OPS activities. Enable the multi_pool_activities test. Change-Id: I6f6feaedecc9c36285bea975caf24dbf8f5f624b --- inc/roctracer_hip.h | 4 -- src/roctracer/loader.h | 45 ++++++++++--------- src/roctracer/roctracer.cpp | 13 ++---- test/CMakeLists.txt | 6 +++ test/directed/multi_pool_activities.cpp | 5 ++- .../multi_pool_activities_trace.txt | 30 +++++++++++++ test/golden_traces/tests_trace_cmp_levels.txt | 1 + test/run.sh | 1 + 8 files changed, 70 insertions(+), 35 deletions(-) create mode 100644 test/golden_traces/multi_pool_activities_trace.txt diff --git a/inc/roctracer_hip.h b/inc/roctracer_hip.h index 0510ac7774..5ac2d1a7cf 100644 --- a/inc/roctracer_hip.h +++ b/inc/roctracer_hip.h @@ -50,10 +50,6 @@ extern "C" { // Traced calls ID enumeration typedef enum hip_api_id_t roctracer_hip_api_cid_t; -typedef void(hipInitAsyncActivityCallback_t)(void* op_callback, void* arg); -typedef bool(hipEnableAsyncActivityCallback_t)(unsigned op, bool enable); -typedef const char*(hipGetOpName_t)(unsigned op); - #ifdef __cplusplus } // extern "C" block #endif // __cplusplus diff --git a/src/roctracer/loader.h b/src/roctracer/loader.h index 5d1adcc731..8b097bc26b 100644 --- a/src/roctracer/loader.h +++ b/src/roctracer/loader.h @@ -151,9 +151,13 @@ __attribute__((weak)) const char* hipKernelNameRefByPtr(const void* hostFunction __attribute__((weak)) int hipGetStreamDeviceId(hipStream_t stream) { return 0; } __attribute__((weak)) const char* hipApiName(uint32_t id) { return NULL; } -__attribute__((weak)) void hipInitActivityCallback(void* id_callback, void* op_callback, - void* arg) {} -__attribute__((weak)) bool hipEnableActivityCallback(unsigned op, bool enable) { return false; } +__attribute__((weak)) hipError_t hipRegisterAsyncActivityCallback_t(uint32_t op, void* fun, + void* arg) { + return hipErrorUnknown; +} +__attribute__((weak)) hipError_t hipRemoveAsyncActivityCallback_t(uint32_t op) { + return hipErrorUnknown; +} __attribute__((weak)) const char* hipGetCmdName(unsigned op) { return NULL; } class HipLoaderStatic { @@ -183,8 +187,12 @@ class HipLoaderStatic { GetStreamDeviceId_t* GetStreamDeviceId; ApiName_t* ApiName; - hipInitAsyncActivityCallback_t* InitActivityCallback; - hipEnableAsyncActivityCallback_t* EnableActivityCallback; + typedef hipError_t(hipRegisterAsyncActivityCallback_t)(uint32_t op, void* fun, void* arg); + typedef hipError_t(hipRemoveAsyncActivityCallback_t)(uint32_t op); + typedef const char*(hipGetOpName_t)(unsigned op); + + hipRegisterAsyncActivityCallback_t* RegisterActivityCallback; + hipRemoveAsyncActivityCallback_t* RemoveActivityCallback; hipGetOpName_t* GetOpName; static inline loader_t& Instance() { @@ -200,7 +208,6 @@ class HipLoaderStatic { } bool Enabled() const { return true; } - bool& InitActivityDone() { return init_activity_done_; } private: HipLoaderStatic() { @@ -213,14 +220,13 @@ class HipLoaderStatic { GetStreamDeviceId = hipGetStreamDeviceId; ApiName = hipApiName; - InitActivityCallback = hipInitActivityCallback; - EnableActivityCallback = hipEnableActivityCallback; + RegisterAsyncActivityCallback = hipRegisterAsyncActivityCallback; + RemoveAsyncActivityCallback = hipRemoveAsyncActivityCallback; GetOpName = hipGetCmdName; } static mutex_t mutex_; static instance_t instance_; - bool init_activity_done_ = false; }; #else class HipApi { @@ -248,11 +254,13 @@ class HipApi { GetStreamDeviceId_t* GetStreamDeviceId; ApiName_t* ApiName; - hipInitAsyncActivityCallback_t* InitActivityCallback; - hipEnableAsyncActivityCallback_t* EnableActivityCallback; - hipGetOpName_t* GetOpName; + typedef hipError_t(hipRegisterAsyncActivityCallback_t)(uint32_t op, void* fun, void* arg); + typedef hipError_t(hipRemoveAsyncActivityCallback_t)(uint32_t op); + typedef const char*(hipGetOpName_t)(unsigned op); - bool& InitActivityDone() { return init_activity_done_; } + hipRegisterAsyncActivityCallback_t* RegisterAsyncActivityCallback; + hipRemoveAsyncActivityCallback_t* RemoveAsyncActivityCallback; + hipGetOpName_t* GetOpName; protected: void init(Loader* loader) { @@ -266,15 +274,12 @@ class HipApi { GetStreamDeviceId = loader->GetFun("hipGetStreamDeviceId"); ApiName = loader->GetFun("hipApiName"); - InitActivityCallback = - loader->GetFun("hipInitActivityCallback"); - EnableActivityCallback = - loader->GetFun("hipEnableActivityCallback"); + RegisterAsyncActivityCallback = + loader->GetFun("hipRegisterAsyncActivityCallback"); + RemoveAsyncActivityCallback = + loader->GetFun("hipRemoveAsyncActivityCallback"); GetOpName = loader->GetFun("hipGetCmdName"); } - - private: - bool init_activity_done_ = false; }; #endif diff --git a/src/roctracer/roctracer.cpp b/src/roctracer/roctracer.cpp index d77d600fdf..71a8c4f66a 100644 --- a/src/roctracer/roctracer.cpp +++ b/src/roctracer/roctracer.cpp @@ -698,14 +698,9 @@ static void roctracer_enable_activity_fun(roctracer_domain_t domain, uint32_t op RocpLoader::Instance(); break; case ACTIVITY_DOMAIN_HIP_OPS: { - if (!HipLoader::Instance().Enabled()) break; - std::lock_guard lock(hip_activity_mutex); - - if (!HipLoader::Instance().InitActivityDone()) { - HipLoader::Instance().InitActivityCallback((void*)HIP_AsyncActivityCallback, pool); - HipLoader::Instance().InitActivityDone() = true; - } - if (!HipLoader::Instance().EnableActivityCallback(op, true)) + if (HipLoader::Instance().Enabled() && + HipLoader::Instance().RegisterAsyncActivityCallback(op, (void*)HIP_AsyncActivityCallback, + pool) != hipSuccess) FATAL_LOGGING("HIP::EnableActivityCallback error"); break; } @@ -800,7 +795,7 @@ static void roctracer_disable_activity_fun(roctracer_domain_t domain, uint32_t o break; case ACTIVITY_DOMAIN_HIP_OPS: { if (HipLoader::Instance().Enabled() && - !HipLoader::Instance().EnableActivityCallback(op, false)) + HipLoader::Instance().RemoveAsyncActivityCallback(op) != hipSuccess) FATAL_LOGGING("HIP::EnableActivityCallback(nullptr) error, op(" << op << ")"); break; } diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 91b35b54e1..b38c459b4c 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -139,6 +139,12 @@ hip_add_executable(activity_and_callback directed/activity_and_callback.cpp) target_link_libraries(activity_and_callback roctracer) add_dependencies(mytest activity_and_callback) +## Build the multi_pool_activities test +set_source_files_properties(directed/multi_pool_activities.cpp PROPERTIES HIP_SOURCE_PROPERTY_FORMAT 1) +hip_add_executable(multi_pool_activities directed/multi_pool_activities.cpp) +target_link_libraries(multi_pool_activities roctracer) +add_dependencies(mytest multi_pool_activities) + ## Copy the golden traces and test scripts configure_file(run.sh ${PROJECT_BINARY_DIR} COPYONLY) execute_process(COMMAND ${CMAKE_COMMAND} -E create_symlink run.sh ${PROJECT_BINARY_DIR}/run_ci.sh) diff --git a/test/directed/multi_pool_activities.cpp b/test/directed/multi_pool_activities.cpp index c8a5ca720d..b948b47820 100644 --- a/test/directed/multi_pool_activities.cpp +++ b/test/directed/multi_pool_activities.cpp @@ -67,7 +67,7 @@ int main() { roctracer_properties_t properties{}; properties.buffer_callback_fun = buffer_callback; properties.buffer_callback_arg = nullptr; - properties.buffer_size = 1024; + properties.buffer_size = 1024 * 1024; roctracer_pool_t* pool_1; CHECK(roctracer_open_pool_expl(&properties, &pool_1)); @@ -76,6 +76,7 @@ int main() { roctracer_pool_t* pool_2; CHECK(roctracer_open_pool_expl(&properties, &pool_2)); CHECK(roctracer_enable_op_activity_expl(ACTIVITY_DOMAIN_HIP_OPS, HIP_OP_ID_COPY, pool_2)); + CHECK(roctracer_enable_op_activity_expl(ACTIVITY_DOMAIN_HIP_API, HIP_API_ID_hipMemcpy, pool_2)); int host_array[256] = {0}; int* device_memory; @@ -90,4 +91,4 @@ int main() { CHECK(roctracer_flush_activity_expl(pool_1)); CHECK(roctracer_flush_activity_expl(pool_2)); return 0; -} \ No newline at end of file +} diff --git a/test/golden_traces/multi_pool_activities_trace.txt b/test/golden_traces/multi_pool_activities_trace.txt new file mode 100644 index 0000000000..fda368860b --- /dev/null +++ b/test/golden_traces/multi_pool_activities_trace.txt @@ -0,0 +1,30 @@ + :KernelExecution : correlation_id(0) time_ns(109660011583441:109660011588241) + :KernelExecution : correlation_id(0) time_ns(109660011855280:109660011859600) + :KernelExecution : correlation_id(0) time_ns(109660011969840:109660011973679) + :KernelExecution : correlation_id(0) time_ns(109660012037039:109660012041199) + :KernelExecution : correlation_id(0) time_ns(109660012081199:109660012084559) + :KernelExecution : correlation_id(0) time_ns(109660012122799:109660012126159) + :KernelExecution : correlation_id(0) time_ns(109660012164399:109660012168239) + :KernelExecution : correlation_id(0) time_ns(109660012206158:109660012209838) + :KernelExecution : correlation_id(0) time_ns(109660012247118:109660012250478) + :KernelExecution : correlation_id(0) time_ns(109660012289678:109660012293518) + :CopyHostToDevice : correlation_id(1) time_ns(109660008446578:109660008452178) + :hipMemcpy : correlation_id(1) time_ns(109659777462237:109660008474607) + :CopyHostToDevice : correlation_id(2) time_ns(109660011646881:109660011651041) + :hipMemcpy : correlation_id(2) time_ns(109660011115400:109660011817555) + :CopyHostToDevice : correlation_id(3) time_ns(109660011942080:109660011946240) + :hipMemcpy : correlation_id(3) time_ns(109660011846359:109660011951538) + :CopyHostToDevice : correlation_id(4) time_ns(109660011985759:109660011989919) + :hipMemcpy : correlation_id(4) time_ns(109660011961286:109660011994288) + :CopyHostToDevice : correlation_id(5) time_ns(109660012053439:109660012057599) + :hipMemcpy : correlation_id(5) time_ns(109660012029645:109660012062688) + :CopyHostToDevice : correlation_id(6) time_ns(109660012096639:109660012100799) + :hipMemcpy : correlation_id(6) time_ns(109660012073037:109660012105278) + :CopyHostToDevice : correlation_id(7) time_ns(109660012138239:109660012142879) + :hipMemcpy : correlation_id(7) time_ns(109660012114796:109660012147087) + :CopyHostToDevice : correlation_id(8) time_ns(109660012180158:109660012184478) + :hipMemcpy : correlation_id(8) time_ns(109660012156274:109660012188795) + :CopyHostToDevice : correlation_id(9) time_ns(109660012221438:109660012225758) + :hipMemcpy : correlation_id(9) time_ns(109660012198213:109660012230234) + :CopyHostToDevice : correlation_id(10) time_ns(109660012262398:109660012266878) + :hipMemcpy : correlation_id(10) time_ns(109660012239211:109660012271171) diff --git a/test/golden_traces/tests_trace_cmp_levels.txt b/test/golden_traces/tests_trace_cmp_levels.txt index 8a595fede3..439d505105 100644 --- a/test/golden_traces/tests_trace_cmp_levels.txt +++ b/test/golden_traces/tests_trace_cmp_levels.txt @@ -19,5 +19,6 @@ code_obj_trace --check-none trace_buffer --check-none memory_pool --check-none activity_and_callback_trace --check-order .* +multi_pool_activities_trace --check-order .* roctx_test_trace --check-count .* backward_compat_test_trace --check-none diff --git a/test/run.sh b/test/run.sh index 1cf7720e8f..375dea39e5 100755 --- a/test/run.sh +++ b/test/run.sh @@ -184,6 +184,7 @@ unset LD_PRELOAD eval_test "directed TraceBuffer test" ./test/trace_buffer trace_buffer eval_test "directed MemoryPool test" ./test/memory_pool memory_pool eval_test "enable/disable callbacks and activities test" ./test/activity_and_callback activity_and_callback_trace +eval_test "use multiple memory pools in HIP activities test" ./test/multi_pool_activities multi_pool_activities_trace eval_test "backward compatibility tests" ./test/backward_compat_test backward_compat_test_trace