Skip to content

Commit

Permalink
[OpenMP][libomptarget] Fix deinit of NextGen AMDGPU plugin
Browse files Browse the repository at this point in the history
This patch fixes a segfault that was appearing when the plugin fails to
initialize and then is deinitialized. Also, do not call hsa_shut_down if
the hsa_init failed.

Differential Revision: https://reviews.llvm.org/D142145
  • Loading branch information
kevinsala committed Jan 20, 2023
1 parent d148f8d commit 097f426
Showing 1 changed file with 21 additions and 4 deletions.
25 changes: 21 additions & 4 deletions openmp/libomptarget/plugins-nextgen/amdgpu/src/rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2245,7 +2245,9 @@ struct AMDGPUGlobalHandlerTy final : public GenericGlobalHandlerTy {
/// Class implementing the AMDGPU-specific functionalities of the plugin.
struct AMDGPUPluginTy final : public GenericPluginTy {
/// Create an AMDGPU plugin and initialize the AMDGPU driver.
AMDGPUPluginTy() : GenericPluginTy(getTripleArch()), HostDevice(nullptr) {}
AMDGPUPluginTy()
: GenericPluginTy(getTripleArch()), Initialized(false),
HostDevice(nullptr) {}

/// This class should not be copied.
AMDGPUPluginTy(const AMDGPUPluginTy &) = delete;
Expand All @@ -2256,10 +2258,14 @@ struct AMDGPUPluginTy final : public GenericPluginTy {
hsa_status_t Status = hsa_init();
if (Status != HSA_STATUS_SUCCESS) {
// Cannot call hsa_success_string.
DP("Failed initialize AMDGPU's HSA library\n");
DP("Failed to initialize AMDGPU's HSA library\n");
return 0;
}

// The initialization of HSA was successful. It should be safe to call
// HSA functions from now on, e.g., hsa_shut_down.
Initialized = true;

// Register event handler to detect memory errors on the devices.
Status = hsa_amd_register_system_event_handler(eventHandler, nullptr);
if (auto Err = Plugin::check(
Expand Down Expand Up @@ -2319,8 +2325,14 @@ struct AMDGPUPluginTy final : public GenericPluginTy {

/// Deinitialize the plugin.
Error deinitImpl() override {
if (auto Err = HostDevice->deinit())
return Err;
// The HSA runtime was not initialized, so nothing from the plugin was
// actually initialized.
if (!Initialized)
return Plugin::success();

if (HostDevice)
if (auto Err = HostDevice->deinit())
return Err;

// Finalize the HSA runtime.
hsa_status_t Status = hsa_shut_down();
Expand Down Expand Up @@ -2427,6 +2439,11 @@ struct AMDGPUPluginTy final : public GenericPluginTy {
return HSA_STATUS_ERROR;
}

/// Indicate whether the HSA runtime was correctly initialized. Even if there
/// is no available devices this boolean will be true. It indicates whether
/// we can safely call HSA functions (e.g., hsa_shut_down).
bool Initialized;

/// Arrays of the available GPU and CPU agents. These arrays of handles should
/// not be here but in the AMDGPUDeviceTy structures directly. However, the
/// HSA standard does not provide API functions to retirve agents directly,
Expand Down

0 comments on commit 097f426

Please sign in to comment.