[Openmp-commits] [openmp] 6760234 - [libomptarget][amdgpu] Precisely manage hsa lifetime

[Jon Chesterfield via Openmp-commits]

Author: Jon Chesterfield
Date: 2021-09-09T17:28:11+01:00
New Revision: 6760234e8d59ef22b816eef087d82f2e116eab08

URL: https://github.com/llvm/llvm-project/commit/6760234e8d59ef22b816eef087d82f2e116eab08
DIFF: https://github.com/llvm/llvm-project/commit/6760234e8d59ef22b816eef087d82f2e116eab08.diff

LOG: [libomptarget][amdgpu] Precisely manage hsa lifetime

The hsa library must be initialized before any calls into it and
destructed after the last call into it. There have been a number of bugs in
this area related to member variables which would like to use raii to manage
resources acquired from hsa.

This patch moves the init/shutdown of hsa into a class, such that when used as
the first member variable (could be a base), the lifetime of other member
variables are reliably scoped within it. This will allow other classes to use
raii reliably when used as member variables within the global.

Reviewed By: pdhaliwal

Differential Revision: https://reviews.llvm.org/D109512

Added: 
    

Modified: 
    openmp/libomptarget/plugins/amdgpu/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
index 82bbbad6172c..f72719ad676d 100644
--- a/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
+++ b/openmp/libomptarget/plugins/amdgpu/src/rtl.cpp
@@ -426,10 +426,30 @@ static constexpr const llvm::omp::GV &getGridValue() {
   return llvm::omp::getAMDGPUGridValues<wavesize>();
 }
 
+struct HSALifetime {
+  // Wrapper around HSA used to ensure it is constructed before other types
+  // and destructed after, which means said other types can use raii for
+  // cleanup without risking running outside of the lifetime of HSA
+  const hsa_status_t S;
+
+  bool success() { return S == HSA_STATUS_SUCCESS; }
+  HSALifetime() : S(hsa_init()) {}
+
+  ~HSALifetime() {
+    if (S == HSA_STATUS_SUCCESS) {
+      hsa_status_t Err = hsa_shut_down();
+      if (Err != HSA_STATUS_SUCCESS) {
+        // Can't call into HSA to get a string from the integer
+        DP("Shutting down HSA failed: %d\n", Err);
+      }
+    }
+  }
+};
+
 /// Class containing all the device information
 class RTLDeviceInfoTy {
+  HSALifetime HSA; // First field => constructed first and destructed last
   std::vector<std::list<FuncOrGblEntryTy>> FuncGblEntries;
-  bool HSAInitializeSucceeded = false;
 
 public:
   // load binary populates symbol tables and mutates various global state
@@ -680,24 +700,26 @@ class RTLDeviceInfoTy {
   }
 
   RTLDeviceInfoTy() {
+    DP("Start initializing " GETNAME(TARGET_NAME) "\n");
+
     // LIBOMPTARGET_KERNEL_TRACE provides a kernel launch trace to stderr
     // anytime. You do not need a debug library build.
     //  0 => no tracing
     //  1 => tracing dispatch only
     // >1 => verbosity increase
+
+    if (!HSA.success()) {
+      DP("Error when initializing HSA in " GETNAME(TARGET_NAME) "\n");
+      return;
+    }
+
     if (char *envStr = getenv("LIBOMPTARGET_KERNEL_TRACE"))
       print_kernel_trace = atoi(envStr);
     else
       print_kernel_trace = 0;
 
-    DP("Start initializing " GETNAME(TARGET_NAME) "\n");
-    hsa_status_t err = hsa_init();
-    if (err == HSA_STATUS_SUCCESS) {
-      err = core::atl_init_gpu_context();
-    }
-    if (err == HSA_STATUS_SUCCESS) {
-      HSAInitializeSucceeded = true;
-    } else {
+    hsa_status_t err = core::atl_init_gpu_context();
+    if (err != HSA_STATUS_SUCCESS) {
       DP("Error when initializing " GETNAME(TARGET_NAME) "\n");
       return;
     }
@@ -801,7 +823,7 @@ class RTLDeviceInfoTy {
 
   ~RTLDeviceInfoTy() {
     DP("Finalizing the " GETNAME(TARGET_NAME) " DeviceInfo.\n");
-    if (!HSAInitializeSucceeded) {
+    if (!HSA.success()) {
       // Then none of these can have been set up and they can't be torn down
       return;
     }
@@ -820,12 +842,6 @@ class RTLDeviceInfoTy {
            "Destroying executable", get_error_string(Err));
       }
     }
-
-    Err = hsa_shut_down();
-    if (Err != HSA_STATUS_SUCCESS) {
-      DP("[%s:%d] %s failed: %s\n", __FILE__, __LINE__, "Shutting down HSA",
-         get_error_string(Err));
-    }
   }
 };