From 82bce811eec1bd09f696bf1219d6dccc17e449f4 Mon Sep 17 00:00:00 2001 From: Laurent Morichetti Date: Tue, 30 Aug 2022 17:55:43 -0700 Subject: [PATCH] SWDEV-351980 - Consolidate registration tables in the roctracer library Remove the api_callbacks_table_t that was holding the API activities and user callbacks. Instead use a single roctracer callback (TracerCallback) used to report both API activities and callbacks. Remove the hipInitActivityCallback that was setting the ROCtracer callback and memory pool for asynchronous activities as it did not allow disctinct pools to be used for each activity. Instead, use hipRegisterTracerCallback to set the single roctracer callback. Change-Id: I4c10f04f29a6e4cce8caf15db3016c3f72c86b04 --- hipamd/src/amdhip.def | 7 +- hipamd/src/hip_activity.cpp | 12 +-- hipamd/src/hip_graph_internal.cpp | 4 +- hipamd/src/hip_hcc.def.in | 7 +- hipamd/src/hip_hcc.map.in | 7 +- hipamd/src/hip_intercept.cpp | 45 ++++------ hipamd/src/hip_prof_api.h | 134 +++++++----------------------- hipamd/src/hip_stream.cpp | 11 +-- 8 files changed, 57 insertions(+), 170 deletions(-) diff --git a/hipamd/src/amdhip.def b/hipamd/src/amdhip.def index 9f06a70a36..3cb1b02f80 100644 --- a/hipamd/src/amdhip.def +++ b/hipamd/src/amdhip.def @@ -205,10 +205,7 @@ hipConfigureCall hipSetupArgument hipLaunchByPtr hipLaunchKernel -hipRegisterApiCallback -hipRemoveApiCallback -hipRegisterActivityCallback -hipRemoveActivityCallback +hipRegisterTracerCallback hipApiName hipKernelNameRef hipBindTexture @@ -254,8 +251,6 @@ hipProfilerStart hipProfilerStop hipCreateSurfaceObject hipDestroySurfaceObject -hipInitActivityCallback -hipEnableActivityCallback hipGetCmdName hipMipmappedArrayCreate hipMallocMipmappedArray diff --git a/hipamd/src/hip_activity.cpp b/hipamd/src/hip_activity.cpp index d96a0bb734..a13a33126b 100644 --- a/hipamd/src/hip_activity.cpp +++ b/hipamd/src/hip_activity.cpp @@ -19,16 +19,8 @@ THE SOFTWARE. */ #include "platform/activity.hpp" - -extern "C" void hipInitActivityCallback(void* op_callback, void* arg) { - activity_prof::CallbacksTable::init(reinterpret_cast(op_callback), - arg); -} - -extern "C" bool hipEnableActivityCallback(unsigned op, bool enable) { - return activity_prof::CallbacksTable::setEnabled(static_cast(op), enable); -} +#include extern "C" const char* hipGetCmdName(unsigned op) { return getOclCommandKindString(static_cast(op)); -} +} \ No newline at end of file diff --git a/hipamd/src/hip_graph_internal.cpp b/hipamd/src/hip_graph_internal.cpp index e3df8a1000..38c00abb5c 100644 --- a/hipamd/src/hip_graph_internal.cpp +++ b/hipamd/src/hip_graph_internal.cpp @@ -689,10 +689,8 @@ hipError_t hipGraphExec::CreateQueues(size_t numQueues) { parallelQueues_.reserve(numQueues); for (size_t i = 0; i < numQueues; i++) { amd::HostQueue* queue; - cl_command_queue_properties properties = - callbacks_table.is_enabled() ? CL_QUEUE_PROFILING_ENABLE : 0; queue = new amd::HostQueue(*hip::getCurrentDevice()->asContext(), - *hip::getCurrentDevice()->devices()[0], properties, + *hip::getCurrentDevice()->devices()[0], 0, amd::CommandQueue::RealTimeDisabled, amd::CommandQueue::Priority::Normal); bool result = (queue != nullptr) ? queue->create() : false; diff --git a/hipamd/src/hip_hcc.def.in b/hipamd/src/hip_hcc.def.in index 469d99d1aa..97b85a2656 100644 --- a/hipamd/src/hip_hcc.def.in +++ b/hipamd/src/hip_hcc.def.in @@ -205,10 +205,7 @@ hipConfigureCall hipSetupArgument hipLaunchByPtr hipLaunchKernel -hipRegisterApiCallback -hipRemoveApiCallback -hipRegisterActivityCallback -hipRemoveActivityCallback +hipRegisterTracerCallback hipApiName hipKernelNameRef hipBindTexture @@ -254,8 +251,6 @@ hipProfilerStart hipProfilerStop hipCreateSurfaceObject hipDestroySurfaceObject -hipInitActivityCallback -hipEnableActivityCallback hipGetCmdName hiprtcAddNameExpression hiprtcCompileProgram diff --git a/hipamd/src/hip_hcc.map.in b/hipamd/src/hip_hcc.map.in index 965321884b..33cae0064c 100644 --- a/hipamd/src/hip_hcc.map.in +++ b/hipamd/src/hip_hcc.map.in @@ -187,10 +187,6 @@ global: hipSetupArgument; hipLaunchByPtr; hipLaunchKernel; - hipRegisterApiCallback; - hipRemoveApiCallback; - hipRegisterActivityCallback; - hipRemoveActivityCallback; hipApiName; hipKernelNameRef; hipKernelNameRefByPtr; @@ -272,8 +268,6 @@ global: hipDestroySurfaceObject*; hipHccModuleLaunchKernel*; hipExtModuleLaunchKernel*; - hipInitActivityCallback*; - hipEnableActivityCallback*; hipGetCmdName*; hipExtStreamCreateWithCUMask; hipStreamGetPriority; @@ -502,6 +496,7 @@ global: hipGraphReleaseUserObject; hipLaunchHostFunc; hipLaunchHostFunc_spt; + hipRegisterTracerCallback; local: *; } hip_5.2; diff --git a/hipamd/src/hip_intercept.cpp b/hipamd/src/hip_intercept.cpp index 9fbc2f66c5..d5a2af10ae 100644 --- a/hipamd/src/hip_intercept.cpp +++ b/hipamd/src/hip_intercept.cpp @@ -25,50 +25,41 @@ // HIP API callback/activity -api_callbacks_table_t callbacks_table; - extern const std::string& FunctionName(const hipFunction_t f); extern "C" { -const char* hipKernelNameRef(const hipFunction_t f) { return FunctionName(f).c_str(); } - int hipGetStreamDeviceId(hipStream_t stream) { if (!hip::isValid(stream)) { return -1; } hip::Stream* s = reinterpret_cast(stream); - return (s != nullptr)? s->DeviceId() : ihipGetDevice(); + return (s != nullptr) ? s->DeviceId() : ihipGetDevice(); } -const char* hipKernelNameRefByPtr(const void* hostFunction, hipStream_t stream) { - [](auto&&...){}(stream); - if (hostFunction == nullptr) { - return nullptr; - } - return PlatformState::instance().getStatFuncName(hostFunction); +const char* hipKernelNameRef(const hipFunction_t function) { + return FunctionName(function).c_str(); } -hipError_t hipRegisterApiCallback(uint32_t id, void* fun, void* arg) { - return callbacks_table.set_callback(static_cast(id), reinterpret_cast(fun), arg) ? - hipSuccess : hipErrorInvalidValue; +const char* hipKernelNameRefByPtr(const void* host_function, hipStream_t stream) { + return (host_function != nullptr) ? PlatformState::instance().getStatFuncName(host_function) + : nullptr; } -hipError_t hipRemoveApiCallback(uint32_t id) { - return callbacks_table.set_callback(static_cast(id), nullptr, nullptr) ? hipSuccess : hipErrorInvalidValue; +void hipRegisterTracerCallback(const void* function) { + activity_prof::report_activity.store( + reinterpret_cast(function), + std::memory_order_relaxed); } +const char* hipApiName(uint32_t id) { return hip_api_name(id); } + +// Deprecated functions that need to be removed from hip_runtime_api.h +hipError_t hipRegisterApiCallback(uint32_t id, void* fun, void* arg) { return hipErrorUnknown; } +hipError_t hipRemoveApiCallback(uint32_t id) { return hipErrorUnknown; } hipError_t hipRegisterActivityCallback(uint32_t id, void* fun, void* arg) { - return callbacks_table.set_activity(static_cast(id), reinterpret_cast(fun), arg) ? - hipSuccess : hipErrorInvalidValue; + return hipErrorUnknown; } +hipError_t hipRemoveActivityCallback(uint32_t id) { return hipErrorUnknown; } -hipError_t hipRemoveActivityCallback(uint32_t id) { - return callbacks_table.set_activity(static_cast(id), nullptr, nullptr) ? hipSuccess : hipErrorInvalidValue; -} - -const char* hipApiName(uint32_t id) { - return hip_api_name(id); -} - -} // extern "C" +} // extern "C" diff --git a/hipamd/src/hip_prof_api.h b/hipamd/src/hip_prof_api.h index 38a50584c5..c552ee5ebc 100644 --- a/hipamd/src/hip_prof_api.h +++ b/hipamd/src/hip_prof_api.h @@ -31,131 +31,61 @@ #include "hip/amd_detail/hip_prof_str.h" #include "platform/prof_protocol.h" -// HIP API callbacks spawner object macro -#define HIP_CB_SPAWNER_OBJECT(CB_ID) \ - api_callbacks_spawner_t \ - __api_tracer([=](auto &api_data) constexpr { INIT_CB_ARGS_DATA(CB_ID, api_data); }); +struct hip_api_trace_data_t { + hip_api_data_t api_data; + uint64_t phase_enter_timestamp; + uint64_t phase_data; -class api_callbacks_table_t { - public: - api_callbacks_table_t() = default; - - bool set_activity(hip_api_id_t id, activity_sync_callback_t function, void* arg) { - if (id < HIP_API_ID_FIRST || id > HIP_API_ID_LAST) - return false; - - auto& entry = callbacks_table_[id]; - std::unique_lock lock(entry.mutex); - - /* 'function != nullptr' indicates it is activity register call, - increment should happen only once but client is free to call - register CB multiple times for same API id hence the check - - 'function == nullptr' indicates it is de-register call and - decrement should happen only once hence the check. */ - - if (function != nullptr) { - if (entry.activity.first == nullptr) - enabled_api_count_.fetch_add(1, std::memory_order_relaxed); - } else { - if (entry.activity.first != nullptr) - enabled_api_count_.fetch_sub(1, std::memory_order_relaxed); - } - entry.activity = {function, arg}; - - return true; - } - - bool set_callback(hip_api_id_t id, activity_rtapi_callback_t function, void* arg) { - if (id < HIP_API_ID_FIRST || id > HIP_API_ID_LAST) - return false; - - auto& entry = callbacks_table_[id]; - std::unique_lock lock(entry.mutex); - entry.user_callback = {function, arg}; - - return true; - } - - auto get(hip_api_id_t id) { - assert(id >= HIP_API_ID_FIRST && id <= HIP_API_ID_LAST && "invalid callback id"); - - auto& entry = callbacks_table_[id]; - std::shared_lock lock(entry.mutex); - return std::make_pair(entry.user_callback, entry.activity); - } - - bool is_enabled() const { - return enabled_api_count_.load(std::memory_order_relaxed) > 0; - } - - private: - std::atomic enabled_api_count_{0}; - - // HIP API callbacks table - struct { - std::shared_mutex mutex; - std::pair activity; - std::pair user_callback; - } callbacks_table_[HIP_API_ID_LAST + 1]{}; + void (*phase_enter)(hip_api_id_t operation_id, hip_api_trace_data_t* data); + void (*phase_exit)(hip_api_id_t operation_id, hip_api_trace_data_t* data); }; -extern api_callbacks_table_t callbacks_table; +// HIP API callbacks spawner object macro +#define HIP_CB_SPAWNER_OBJECT(operation_id) \ + api_callbacks_spawner_t __api_tracer( \ + [=](auto& api_data) { INIT_CB_ARGS_DATA(operation_id, api_data); }); -template -class api_callbacks_spawner_t { +template class api_callbacks_spawner_t { public: - template - constexpr api_callbacks_spawner_t(Functor init_cb_args_data) : record_() - { - static_assert(ID >= HIP_API_ID_FIRST && ID <= HIP_API_ID_LAST, "invalid callback id"); - if (!callbacks_table.is_enabled()) return; + template api_callbacks_spawner_t(Functor init_cb_args_data) { + static_assert(operation_id >= HIP_API_ID_FIRST && operation_id <= HIP_API_ID_LAST, + "invalid HIP_API operation id"); - std::tie(user_callback_, activity_) = callbacks_table.get(ID); - if (activity_.first == nullptr) - return; + if (auto function = activity_prof::report_activity.load(std::memory_order_relaxed); function && + (enabled_ = function(ACTIVITY_DOMAIN_HIP_API, operation_id, &trace_data_) == 0)) { + activity_prof::correlation_id = trace_data_.api_data.correlation_id; - api_data_.phase = ACTIVITY_API_PHASE_ENTER; - activity_.first(ID, &record_, &api_data_, activity_.second); - activity_prof::correlation_id = api_data_.correlation_id; - - if (user_callback_.first) { - init_cb_args_data(api_data_); - user_callback_.first(ACTIVITY_DOMAIN_HIP_API, ID, &api_data_, user_callback_.second); + if (trace_data_.phase_enter != nullptr) { + init_cb_args_data(trace_data_.api_data); + trace_data_.phase_enter(operation_id, &trace_data_); + } } } ~api_callbacks_spawner_t() { - if (activity_.first == nullptr) - return; - - api_data_.phase = ACTIVITY_API_PHASE_EXIT; - if (user_callback_.first != nullptr) - user_callback_.first(ACTIVITY_DOMAIN_HIP_API, ID, &api_data_, user_callback_.second); - - activity_prof::correlation_id = 0; - activity_.first(ID, &record_, &api_data_, activity_.second); + if (enabled_) { + if (trace_data_.phase_exit != nullptr) trace_data_.phase_exit(operation_id, &trace_data_); + activity_prof::correlation_id = 0; + } } private: - std::pair user_callback_; - std::pair activity_; - activity_record_t record_; + bool enabled_{false}; union { - hip_api_data_t api_data_; + hip_api_trace_data_t trace_data_; }; }; -template <> -class api_callbacks_spawner_t { +template <> class api_callbacks_spawner_t { public: - template - api_callbacks_spawner_t(Functor) {} + template api_callbacks_spawner_t(Functor) {} }; #else -#define HIP_CB_SPAWNER_OBJECT(x) do {} while(false) +#define HIP_CB_SPAWNER_OBJECT(x) \ + do { \ + } while (false) class api_callbacks_table_t { public: diff --git a/hipamd/src/hip_stream.cpp b/hipamd/src/hip_stream.cpp index 5e51a1ebd2..72708c93b6 100644 --- a/hipamd/src/hip_stream.cpp +++ b/hipamd/src/hip_stream.cpp @@ -24,8 +24,6 @@ #include "thread/monitor.hpp" #include "hip_prof_api.h" -extern api_callbacks_table_t callbacks_table; - static amd::Monitor streamSetLock{"Guards global stream set"}; static std::unordered_set streamSet; namespace hip { @@ -77,12 +75,6 @@ hipError_t Stream::EndCapture() { } // ================================================================================================ bool Stream::Create() { - // Enable queue profiling if a profiler is attached which sets the callback_table flag - // or if we force it with env var. This would enable time stamp collection for every - // command submitted to the stream(queue). - bool isProfilerAttached = callbacks_table.is_enabled(); - cl_command_queue_properties properties = isProfilerAttached ? - CL_QUEUE_PROFILING_ENABLE : 0; amd::CommandQueue::Priority p; switch (priority_) { case Priority::High: @@ -97,7 +89,7 @@ bool Stream::Create() { break; } amd::HostQueue* queue = new amd::HostQueue(*device_->asContext(), *device_->devices()[0], - properties, amd::CommandQueue::RealTimeDisabled, + 0, amd::CommandQueue::RealTimeDisabled, p, cuMask_); // Create a host queue @@ -107,7 +99,6 @@ bool Stream::Create() { amd::ScopedLock lock(streamSetLock); streamSet.insert(this); queue_ = queue; - queue->vdev()->profilerAttach(isProfilerAttached); device_->SaveQueue(queue); } else if (queue != nullptr) { // Queue creation has failed, and virtual device associated with the queue may not be created.