From 85fd9692532da9cbe4a45042d75d667159fb186b Mon Sep 17 00:00:00 2001
From: Laurent Morichetti <laurent.morichetti@amd.com>
Date: Thu, 4 Aug 2022 10:19:31 -0700
Subject: [PATCH] SWDEV-351980 - Move hip_api_data and record to the HIP
 function's stack

Since the hip_api_data and record are only needed at the HIP function's
scope, there is no need to allocate/free them in the ROCtracer activity
callback, they can reside on the HIP function's stack frame.

This solves an issue with the thread local stacks of records the tracer
maintains that are destroyed first (before any global destructor) on
process exit, making it impossible to use HIP functions in global
destructors when the profiler is enabled.

Change-Id: Ib1d70124d009a44dc1f08d41edff95e5f9f84369
---
 hipamd/src/hip_activity.cpp |  9 +++---
 hipamd/src/hip_prof_api.h   | 62 ++++++++++++++++++-------------------
 2 files changed, 34 insertions(+), 37 deletions(-)

diff --git a/hipamd/src/hip_activity.cpp b/hipamd/src/hip_activity.cpp
index 6e1703f65b..d96a0bb734 100644
--- a/hipamd/src/hip_activity.cpp
+++ b/hipamd/src/hip_activity.cpp
@@ -20,16 +20,15 @@
 
 #include "platform/activity.hpp"
 
-extern "C" void hipInitActivityCallback(void* id_callback, void* op_callback, void* arg) {
-  activity_prof::CallbacksTable::init(reinterpret_cast<activity_prof::id_callback_fun_t>(id_callback),
-                                      reinterpret_cast<activity_prof::callback_fun_t>(op_callback),
+extern "C" void hipInitActivityCallback(void* op_callback, void* arg) {
+  activity_prof::CallbacksTable::init(reinterpret_cast<activity_async_callback_t>(op_callback),
                                       arg);
 }
 
 extern "C" bool hipEnableActivityCallback(unsigned op, bool enable) {
-  return activity_prof::CallbacksTable::SetEnabled(op, enable);
+  return activity_prof::CallbacksTable::setEnabled(static_cast<OpId>(op), enable);
 }
 
 extern "C" const char* hipGetCmdName(unsigned op) {
-  return getOclCommandKindString(static_cast<uint32_t>(op));
+  return getOclCommandKindString(static_cast<cl_command_type>(op));
 }
diff --git a/hipamd/src/hip_prof_api.h b/hipamd/src/hip_prof_api.h
index d61cdd0179..464a4ee97b 100644
--- a/hipamd/src/hip_prof_api.h
+++ b/hipamd/src/hip_prof_api.h
@@ -33,14 +33,8 @@
 
 // HIP API callbacks spawner object macro
 #define HIP_CB_SPAWNER_OBJECT(CB_ID) \
-  api_callbacks_spawner_t<HIP_API_ID_##CB_ID> __api_tracer; \
-  { \
-    hip_api_data_t* api_data = __api_tracer.get_api_data_ptr(); \
-    if (api_data != nullptr) { \
-      INIT_CB_ARGS_DATA(CB_ID, (*api_data)); \
-      __api_tracer.call(); \
-    } \
-  }
+  api_callbacks_spawner_t<HIP_API_ID_##CB_ID> \
+    __api_tracer([=](auto &api_data) constexpr { INIT_CB_ARGS_DATA(CB_ID, api_data); });
 
 class api_callbacks_table_t {
  public:
@@ -84,7 +78,7 @@ class api_callbacks_table_t {
     return true;
   }
 
-  auto get_callback_and_activity(hip_api_id_t id) {
+  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];
 
@@ -118,56 +112,60 @@ extern api_callbacks_table_t callbacks_table;
 template <hip_api_id_t ID>
 class api_callbacks_spawner_t {
  public:
-  api_callbacks_spawner_t() :
-    api_data_(nullptr)
+  template <typename Functor>
+  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;
 
-    std::tie(user_callback_, activity_) = callbacks_table.get_callback_and_activity(ID);
+    std::tie(user_callback_, activity_) = callbacks_table.get(ID);
+    if (activity_.first == nullptr)
+      return;
 
-    if (activity_.first != nullptr)
-      api_data_ = (hip_api_data_t*) activity_.first(ID, nullptr, nullptr, nullptr);
-  }
+    api_data_.correlation_id = 0;
+    api_data_.phase = ACTIVITY_API_PHASE_ENTER;
+    activity_.first(ID, &record_, &api_data_, activity_.second);
 
-  void call() {
-    if (user_callback_.first != nullptr)
-      user_callback_.first(ACTIVITY_DOMAIN_HIP_API, ID, api_data_, user_callback_.second);
+    if (user_callback_.first) {
+      init_cb_args_data(api_data_);
+      user_callback_.first(ACTIVITY_DOMAIN_HIP_API, ID, &api_data_, user_callback_.second);
+    }
+
+    activity_prof::correlation_id = api_data_.correlation_id;
   }
 
   ~api_callbacks_spawner_t() {
-    if (api_data_ == nullptr)
+    if (activity_.first == nullptr)
       return;
 
-    api_data_->phase = ACTIVITY_API_PHASE_EXIT;
+    activity_prof::correlation_id = 0;
+
+    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);
+      user_callback_.first(ACTIVITY_DOMAIN_HIP_API, ID, &api_data_, user_callback_.second);
 
-    if (activity_.first != nullptr)
-      activity_.first(ID, nullptr, nullptr, activity_.second);
-  }
-
-  hip_api_data_t* get_api_data_ptr() const {
-    return api_data_;
+    activity_.first(ID, &record_, &api_data_, activity_.second);
   }
 
  private:
   std::pair<activity_rtapi_callback_t /* function */, void * /* arg */> user_callback_;
   std::pair<activity_sync_callback_t /* function */, void * /* arg */> activity_;
-  hip_api_data_t* api_data_;
+  activity_record_t record_;
+  union {
+    hip_api_data_t api_data_;
+  };
 };
 
 template <>
 class api_callbacks_spawner_t<HIP_API_ID_NONE> {
  public:
-  api_callbacks_spawner_t() {}
-  void call() {}
-  hip_api_data_t* get_api_data_ptr() { return nullptr; }
+  template <typename Functor>
+  api_callbacks_spawner_t(Functor) {}
 };
 
 #else
 
-#define HIP_CB_SPAWNER_OBJECT(x) do {} while(0)
+#define HIP_CB_SPAWNER_OBJECT(x) do {} while(false)
 
 class api_callbacks_table_t {
  public: