[libomptarget][amdgpu] More robust handling of failure to init HSA

[JonChesterfield]

Summary

If hsa_init fails, subsequent calls into hsa are not safe. Except for
hsa_init, but we don't retry on failure.

This patch:

• deletes a print that called into hsa to ask why it can't call into hsa
• drops a merge conflict block next to that print
• reliably initializes number of devices to zero
• skips the plugin destructor contents if the constructor failed to init hsa

Tested by making hsa_init return error, and by forcing the dynamic library
use which was then deleted from disk. Before this patch, both segv. After it,
friendly message about offloading being unavailable.

[JonChesterfield]

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

977

This paranoia may be unwarranted - calling __tgt_rtl_number_of_devices will return zero, so the library shouldn't attempt to initialize any. Will drop this sanity check if preferred.

[jdoerfert]

LG

[JonChesterfield]

Landed without the bounds check on device_id in __tgt_rtl_init_device. We might want to check that device_id is non-negative and less than the claimed number of devices, but if so, it should probably be on every external function, and possibly only in debug mode.

openmp/libomptarget/plugins/amdgpu/impl/system.cpp

static hsa_status_t init_compute_and_memory() {
   else
     return HSA_STATUS_ERROR_NOT_INITIALIZED;
 }
 
 hsa_status_t init_hsa() {
   DEBUG_PRINT("Initializing HSA...");
   hsa_status_t err = hsa_init();
   if (err != HSA_STATUS_SUCCESS) {
-    printf("[%s:%d] %s failed: %s\n", __FILE__, __LINE__,
-           "Initializing the hsa runtime", get_error_string(err));
     return err;
   }
-  if (err != HSA_STATUS_SUCCESS)
-    return err;
 
   err = init_compute_and_memory();
   if (err != HSA_STATUS_SUCCESS)
     return err;
   if (err != HSA_STATUS_SUCCESS) {
     printf("[%s:%d] %s failed: %s\n", __FILE__, __LINE__,
            "After initializing compute and memory", get_error_string(err));
     return err;

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

 //===----------------------------------------------------------------------===//
 
 #include <algorithm>
 #include <assert.h>
 #include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <functional>
 #include <libelf.h>
 #include <list>
 #include <memory>
 #include <mutex>
 #include <shared_mutex>
 #include <unordered_map>
 #include <vector>
 
 // Header from ATMI interface
struct EnvironmentVariables {
   int TeamLimit;
   int TeamThreadLimit;
   int MaxTeamsDefault;
 };
 
 /// Class containing all the device information
 class RTLDeviceInfoTy {
   std::vector<std::list<FuncOrGblEntryTy>> FuncGblEntries;
+  bool HSAInitializeSucceeded = false;
 
 public:
   // load binary populates symbol tables and mutates various global state
   // run uses those symbol tables
   std::shared_timed_mutex load_run_lock;
 
-  int NumberOfDevices;
+  int NumberOfDevices = 0;
 
   // GPU devices
   std::vector<hsa_agent_t> HSAAgents;
   std::vector<hsa_queue_t *> HSAQueues; // one per gpu
 
   // CPUs
   std::vector<hsa_agent_t> CPUAgents;
 
RTLDeviceInfoTy() {
     // >1 => verbosity increase
     if (char *envStr = getenv("LIBOMPTARGET_KERNEL_TRACE"))
       print_kernel_trace = atoi(envStr);
     else
       print_kernel_trace = 0;
 
     DP("Start initializing HSA-ATMI\n");
     hsa_status_t err = core::atl_init_gpu_context();
-    if (err != HSA_STATUS_SUCCESS) {
+    if (err == HSA_STATUS_SUCCESS) {
+      HSAInitializeSucceeded = true;
+    } else {
       DP("Error when initializing HSA-ATMI\n");
       return;
     }
 
     // Init hostcall soon after initializing ATMI
     hostrpc_init();
 
     err = FindAgents([&](hsa_device_type_t DeviceType, hsa_agent_t Agent) {
RTLDeviceInfoTy() {
     Env.TeamThreadLimit = readEnvElseMinusOne("OMP_TEAMS_THREAD_LIMIT");
 
     // Default state.
     RequiresFlags = OMP_REQ_UNDEFINED;
   }
 
   ~RTLDeviceInfoTy() {
     DP("Finalizing the HSA-ATMI DeviceInfo.\n");
+    if (!HSAInitializeSucceeded) {
+      // Then none of these can have been set up and they can't be torn down
+      return;
+    }
     // Run destructors on types that use HSA before
     // atmi_finalize removes access to it
     deviceStateStore.clear();
     KernelArgPoolMap.clear();
     // Terminate hostrpc before finalizing ATMI
     hostrpc_terminate();
 
     hsa_status_t Err;
template <typename T> bool enforce_upper_bound(T *value, T upper) {
   }
   return changed;
 }
 } // namespace
 
 int32_t __tgt_rtl_init_device(int device_id) {
   hsa_status_t err;
 
   // this is per device id init
   DP("Initialize the device id: %d\n", device_id);
+  if ((device_id < 0) || (device_id >= DeviceInfo.HSAAgents.size())) {
+    return OFFLOAD_FAIL;
+  }
 
   hsa_agent_t agent = DeviceInfo.HSAAgents[device_id];
 
   // Get number of Compute Unit
   uint32_t compute_units = 0;
   err = hsa_agent_get_info(
       agent, (hsa_agent_info_t)HSA_AMD_AGENT_INFO_COMPUTE_UNIT_COUNT,
       &compute_units);