[OpenMP] [OMPT] [amdgpu] [5/8] Implemented device init/fini/load callbacks

[mhalk]

Summary

Added support in the generic plugin to invoke registered callbacks.

Depends on D124070

Patch from John Mellor-Crummey <[e-mail]>
(With contributions from Dhruva Chakrabarti <[e-mail]>)

[dhruvachak]

Rebased

[jplehr]

openmp/libomptarget/include/ompt_device_callbacks.h

18

Should this be rather <string> or <cstring>?

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

30

I'm not sure about this line. I understand it as it may leak memory, and invalidate all OmptDeviceTy pointers that may have been acquired. At least if this is executed more than once (for whatever reason).

[jplehr]

openmp/libomptarget/include/ompt_device_callbacks.h

53

In other parts I saw that C-style casts were changed to static_cast or reinterpret_cast.
Potentially stick to these C++ casts instead is preferable.

[jplehr]

Rebase / update

[jplehr]

Fixing build error in Debug configuration

[jplehr]

Rebased

[mhalk]

Rebased + related fixes

[mhalk]

Refactored patch for use with nextgen plugins.

[jplehr]

Thanks for moving this forward.

openmp/libomptarget/include/ompt_device_callbacks.h

48

If I recall correctly, methods should be formulated more in active voice, e.g., initializeOmptCallbackDevice, and start with a lower-case letter. I believe this is true for the other methods in this class as well.

136

I think this should be renamed to allocate or so, as the comment suggests that it allocates and does not resize.

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

29 ↗

(On Diff #506967)

Probably better use nullptr here.

32 ↗

(On Diff #506967)

Potentially we should not allocate if Devices already holds a pointer, i.e., is not nullptr? This looks suspiciously as if we can leak memory here.

38 ↗

(On Diff #506967)

Do we want / need to guard against out-of-bounds access here?

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

470–473 ↗

(On Diff #506967)

Idk if there is a guideline for it, but I personally would prefer to have this read as /* FileName= */ nullptr, ...

[mhalk]

When more changes have accumulated I'll update this diff with the changes I replied to with 'Done'.

openmp/libomptarget/include/ompt_device_callbacks.h

48

Agreed.
If more people request this NFC, I'll gladly incorporate this in the patch stack.
I decided against this, since I wanted to concentrate on the core of the patches and not defer them any longer.

136

Agreed. Done.

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

29 ↗

(On Diff #506967)

Done.

32 ↗

(On Diff #506967)

Agreed. I'll simply send a DebugPrint when allocation failed.
That is, when: zero or negative NumDevices were requested or when Devices was already allocated / non-nullptr.

38 ↗

(On Diff #506967)

I think we should.
Done.

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

470–473 ↗

(On Diff #506967)

I would be interested in that, too -- in the previous version of these files it was this (nullptr /* FileName= */,) way.
But TBH I don't find that intuitively either, i.e. I would also prefer your proposal.
Any other thoughts or info on this would be appreciated.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

32 ↗

(On Diff #506967)

Why not assert that Devices is nullptr before allocation?

[mhalk]

Patch rework; implemented feedback.

[mhalk]

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

32 ↗

(On Diff #506967)

I decided to update the revision, so you can see how I handled things "in the meantime".
(Hopefully, my intermediate changes won't break this, otherwise I'll have to update the whole patch stack.)

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

32 ↗

(On Diff #506967)

I would not introduce a new variable for number of devices. I would just add an assert for what we are expecting.

[jdoerfert]

Read the large comment first, this might also result in changes to other patches. The design should match nextgen and it should be a proper part of it, not some attached afterthought.

openmp/libomptarget/include/ompt_device_callbacks.h

142

I don'n understand why there are so many static members and functions everywhere, but alas.

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

30 ↗

(On Diff #507426)

Now there are static members, and static non members, I mean, why?

29 ↗

(On Diff #506967)

Also above?

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

366 ↗

(On Diff #507426)

So, these OmptDevices are stored in 4 or so static members mostly inside OmptDeviceCallbacksTy, and initialized via a member of OmptDeviceCallbacks which is a global object that flies around.

This is really too much.

Let's approach this differently:
One OmptDeviceTy per device of a plugin, right?
If so, make one such object a member of GenericDeviceTy, all that array stuff and 4 functions can be removed, init and deinit are performed via constructor/destructor or init/deinit of the parent.
Funnily, once you do that, the entire OmptDeviceTy code would be moot and all you really put into GenericDeviceTy::(de)init is one conditional to check and call the callback.
You can even make a variadic macro that checks the callback and calls it, forwarding the args, which will simplify the pattern to sth like:

ompt_callback(device_initialize, DeviceNum, Type, reinterpret_cast<ompt_device_t *>(this), doLookup, Documentation);

Then, why do we need OmptDeviceCallbacksTy in the first place?
Shouldn't the callbacks be close to their invocation, e.g., in GenericDeviceTy, etc.

470–473 ↗

(On Diff #506967)

/* Name */ Value, which is hopefully what the rest of the code does.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

366 ↗

(On Diff #507426)

I agree all or most of those statics can go away. There is a single instance of OmptDeviceCallbacksTy in libomptarget and another instance in the plugin. So those statics are redundant.

About OmptDeviceCallbacksTy, I suggest splitting it up into 2 structs. The first, OmptHostEventsCallbacksTy would contain the host callback function ptrs. The second, OmptDeviceEventsCallbacksTy would contain the 4 device callback function ptrs: load, unload, init, and fini. This is along the lines of the categories already present in omp-tools.h. Then, libomptarget would instantiate only OmptHostEventsCallbacksTy by using the "connect" interface to libomp. Similarly, the plugin would instantiate only OmptDeviceEventsCallbacksTy by using the "connect" interface to libomp. (In the current patch, the plugin connects to libomptarget, that will have to be changed.) The object of type OmptDeviceEventsCallbacksTy can reside in GenericDeviceTy. (Later on, when we introduce support for trace records, we will need new OMPT entry points that the tool can call. Examples: ompt_start_trace, ompt_flush_trace, etc. At that point, another struct OmptDeviceEntryPointsTy holding those entry points can be introduced, residing in GenericDeviceTy. But that's for later.) The object of type OmptHostEventsCallbacksTy can stay as a global object like in current trunk unless we find a better place for it.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

1047 ↗

(On Diff #507426)

This will be a problem the way these patches currently are. GenericPluginTy::init() will kick in for multiple plugins, so multiple plugins will initialize the callbacks. Not a problem per se but where it will fail today is during tracking the finalizer in libomptarget. libomptarget is currently set up to track only 1 finalizer.

See openmp/libomptarget/src/OmptCallback.cpp
/// Object that will maintain the finalizer of the plugin
static LibomptargetRtlFinalizer LibOmptTargetRTLFinalizer;

So multiple plugins will have callbacks initialized and the last finalizer will overwrite the rest, not a good thing. (We should actually assert on this condition.)

One solution is to move the call to OmptCallbackInit() within the device-specific plugin implementations, e.g. in AMDGPUPluginTy::initImpl().

[mhalk]

Rebase + implementation of feedback

[mhalk]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

1047 ↗

(On Diff #507426)

Thanks and good catch -- I moved the calls to ompt::connectLibrary(); into the specific amdgpu & cuda plugins.
(This comment will be removed in a following diff update. Oversight.)

[mhalk]

Removed debug prints / comments and fixed a problem when no OMPT was initialized.

[mhalk]

Updated to accomodate the changes made in the previous patch.

[jdoerfert]

openmp/libomptarget/CMakeLists.txt

70 ↗

(On Diff #517683)

?

openmp/libomptarget/include/ompt_connector.h

104 ↗

(On Diff #517683)

?

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

35 ↗

(On Diff #517683)

Style, here and elsewhere. We use LLVM code style in the plugins, at least outside the C-bindings.

openmp/runtime/src/exports_so.txt

29 ↗

(On Diff #517683)

?

openmp/runtime/src/kmp_utility.cpp

410 ↗

(On Diff #517683)

?

[mhalk]

Addressed style issues, dreaded whitespace & removed DEBUG_PREFIX testing

[mhalk]

openmp/libomptarget/CMakeLists.txt

70 ↗

(On Diff #517683)

Had some issues, that was a testing leftover -- sorry!

openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp

35 ↗

(On Diff #517683)

Thanks! Should be addressed (catched three locations).

[mhalk]

This comment was removed by mhalk.

[mhalk]

Update + rebase to reflect the landing of https://reviews.llvm.org/D124070

@jdoerfert If there are any issues left or further improvements that should be implemented, please let me know.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h

27 ↗

(On Diff #519876)

Guard it with OMPT_SUPPORT?

753 ↗

(On Diff #519876)

So now the size of GenericDeviceTy varies based on whether OMPT support is compiled in or not. I would prefer removing the ifdef here and in the ctor (so that proper init happens) so that the size is the same regardless of cmake config. But I am not requesting this change, would be curious what others feel. I just like it that way since it helps debugging when there is a problem.

760 ↗

(On Diff #519876)

Give it a more meaningful name, it's just an atomic boolean now. Some OMPT APIs require an opaque ompt_device_t. In the previous version, we would pass a handle to OmptDevice for those APIs, so that may have to refactored when the time comes.

[dhruvachak]

openmp/libomptarget/src/OmptCallback.cpp

91 ↗

(On Diff #519876)

I thought we got rid of these 2 statics.

[mhalk]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h

27 ↗

(On Diff #519876)

Yes, absolutely.

760 ↗

(On Diff #519876)

Check w.r.t. "API" and I'd tend to sth. like OmptInitialized, since we're already looking at a "Device" this should convey the fact that we're looking at a boolean that's true when this device's OMPT relatied init has been executed.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h

760 ↗

(On Diff #519876)

Ignore the second part of this comment about ompt_device_t. I see what you are doing already in this patch for the opaque handle and that looks good.

[jplehr]

Thanks for moving this forward.

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

386 ↗

(On Diff #519876)

Nit: I believe this should start w/ capital letter.

448 ↗

(On Diff #519876)

captial E

[mhalk]

Update: Removed statics (libomptarget->plugins). Style.

[mhalk]

openmp/libomptarget/src/OmptCallback.cpp

91 ↗

(On Diff #519876)

We wanted to get rid of these statics in openmp/libomptarget/plugins-nextgen/common/OMPT/OmptCallback.cpp
Nevertheless good catch, since those were accidentally re-introduced by a rebase.

AFAIK:
We wanted to keep these here, to make sure the libomp->libomptarget connection stays alive (esp. w.r.t. the result).
But the libomptarget->plugin ones should not be static, since we can have multiple plugins.

[mhalk]

@jdoerfert *ping*

[mhalk]

If there are further requests / improvements I should implement, please let me know.
Otherwise, I'd like to move on with this patch.

Thank you!

[mhalk]

Rebase
Add multiple plugin finalization
Fix cmake not disabling libomptarget OMPT support, when libomp's OMPT support was disabled in the first place

[jplehr]

I know you did quite a bit of testing, so I assume this works. Just the one nit.
LGTM

openmp/libomptarget/src/OmptCallback.cpp

82 ↗

(On Diff #536256)

Should we assert here that LibraryFinalizer is actually nullptr, so we would catch any changes in assumptions about how often something is called early?

[mhalk]

Thank you @jplehr, that is much appreciated.

openmp/libomptarget/src/OmptCallback.cpp

82 ↗

(On Diff #536256)

Yes, I'll add an assert here -- making sure LibraryFinalizer == nullptr before creating the object.

[mhalk]

Added assert w.r.t. LibraryFinalizer == nullptr

[mhalk]

@jdoerfert @dreachem *ping*

In case you have any further feedback.

[dhruvachak]

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

476 ↗

(On Diff #537688)

Is there a reason you need to pass in the GenericPluginTy here? I don't see it getting used.

470–473 ↗

(On Diff #506967)

Yes, that is the guideline. /* Name=*/Value
See https://llvm.org/docs/CodingStandards.html#commenting

openmp/runtime/src/ompt-general.cpp

883 ↗

(On Diff #537688)

This looks unrelated. Can you verify whether we really need this for this patch?

[mhalk]

Thanks @dhruvachak I'll rebase (esp. w.r.t. the deletion of the legacy plugins), then check the upcoming changes and if I don't see problems: submit here.

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

476 ↗

(On Diff #537688)

Very good catch, this *was* needed initally, when the init and fini was done within the generic plugin.
But this change is not necessary anymore as the patch has undergone a lot of changes since then.

openmp/runtime/src/ompt-general.cpp

883 ↗

(On Diff #537688)

Yes, we actually use this in this patch, to provide access to a "callback function lookup by code" (rather than name):
ompt_get_callback_t lookupCallbackByCode
It will be used to bind the actual OMPT callback functions to their declarations.

My idea was: This should provide a lookup with less overhead, than by name (comparing integers vs. strcmp, esp. since the function names are prefixed with ompt_callback_).

[mhalk]

Rebase + reverted signature change of GenericDeviceTy::deinit()

[dhruvachak]

Looks good. Thanks for the changes.

openmp/libomptarget/include/ompt_device_callbacks.h

 //===--------- ompt_device_callbacks.h - OMPT callbacks -- C++ ----------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Interface used by both target-independent and device-dependent runtimes
 // to coordinate registration and invocation of OMPT callbacks
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _OMPT_DEVICE_CALLBACKS_H
 #define _OMPT_DEVICE_CALLBACKS_H
 
+#include <atomic>
+#include <string.h>
+
 #include <omp-tools.h>
 
 #define FOREACH_OMPT_TARGET_CALLBACK(macro)                                    \
   FOREACH_OMPT_DEVICE_EVENT(macro)                                             \
   FOREACH_OMPT_NOEMI_EVENT(macro)                                              \
   FOREACH_OMPT_EMI_EVENT(macro)
 
+/// Internal representation for OMPT device
+class OmptDeviceTy {
+public:
+  OmptDeviceTy() { Enabled.store(false); }
+  bool initialize() {
+    bool Old = false;
+    return Enabled.compare_exchange_strong(Old, true);
+  }
+  bool finalize() {
+    bool Old = true;
+    return Enabled.compare_exchange_strong(Old, false);
+  }
+
+private:
+  std::atomic<bool> Enabled;
+};
+
 /// Internal representation for OMPT device callback functions.
 class OmptDeviceCallbacksTy {
 public:
+  /// Invoked when a device is initialized
+  void OmptCallbackDeviceInitialize(int DeviceNum, const char *Type) {
+    if (ompt_callback_device_initialize_fn) {
+      OmptDeviceTy *Device = lookupDevice(DeviceNum);
+      if (Device && Device->initialize()) {
+        ompt_callback_device_initialize_fn(
+            DeviceNum, Type, (ompt_device_t *)Device, doLookup, Documentation);
+      }
+    }
+  }
+
+  /// Invoked when a device is finalized
+  void OmptCallbackDeviceFinalize(int DeviceNum) {
+    if (ompt_callback_device_finalize_fn) {
+      OmptDeviceTy *Device = lookupDevice(DeviceNum);
+      if (Device && Device->finalize()) {
+        ompt_callback_device_finalize_fn(DeviceNum);
+      }
+    }
+  }
+
+  /// Invoked when a device image is loaded
+  void OmptCallbackDeviceLoad(int DeviceNum, const char *Filename,
+                              int64_t OffsetInFile, void *VmaInFile,
+                              size_t Bytes, void *HostAddr, void *DeviceAddr,
+                              uint64_t ModuleId) {
+    if (ompt_callback_device_load_fn) {
+      ompt_callback_device_load_fn(DeviceNum, Filename, OffsetInFile, VmaInFile,
+                                   Bytes, HostAddr, DeviceAddr, ModuleId);
+    }
+  }
+
+  /// Invoked when a device image is unloaded
+  void OmptCallbackDeviceUnload(int DeviceNum, uint64_t ModuleId) {
+    if (ompt_callback_device_unload_fn) {
+      ompt_callback_device_unload_fn(DeviceNum, ModuleId);
+    }
+  }
+
   /// Initialize the enabled flag and all the callbacks
   void init() {
     Enabled = false;
 #define initName(Name, Type, Code) Name##_fn = 0;
     FOREACH_OMPT_TARGET_CALLBACK(initName)
 #undef initName
   }
 
+  bool isEnabled() { return Enabled; }
+  /// Devices must be prepared before use
+  void prepareDevices(int NumDevices) { resize(NumDevices); }
+  /// Devices must be released after use
+  void releaseDevices() { release(); }
+
   /// Used to register callbacks. \p Lookup is used to query a given callback
   /// by name and the result is assigned to the corresponding callback function.
   void registerCallbacks(ompt_function_lookup_t Lookup) {
     Enabled = true;
 #define OmptBindCallback(Name, Type, Code)                                     \
   Name##_fn = (Name##_t)Lookup(#Name);                                         \
   DP("OMPT: class bound %s=%p\n", #Name, ((void *)(uint64_t)Name##_fn));
 

 private:
   /// Set to true if callbacks for this library have been initialized
   bool Enabled;
 
   /// Callback functions
 #define DeclareName(Name, Type, Code) Name##_t Name##_fn;
   FOREACH_OMPT_TARGET_CALLBACK(DeclareName)
 #undef DeclareName
+
+  /// Allocate devices
+  static void resize(int NumDevices);
+  /// Deallocate devices
+  static void release();
+  /// Find a device given its id
+  static OmptDeviceTy *lookupDevice(int DeviceNum);
+  /// Documentation based on omp-tools
+  static const char *Documentation;
 };
 
 /// Device callbacks object for the library that performs the instantiation
 extern OmptDeviceCallbacksTy ompt_device_callbacks;
 
 #endif

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

 
 #include "Debug.h"
 #include "ompt-connector.h"
 #include "ompt_device_callbacks.h"
 
 /// Object maintaining all the callbacks in the plugin
 OmptDeviceCallbacksTy ompt_device_callbacks;
 
+const char *OmptDeviceCallbacksTy::Documentation = 0;
+
+/// Array denoting the devices
+static OmptDeviceTy *devices = 0;
+
+void OmptDeviceCallbacksTy::resize(int NumDevices) {
+  devices = new OmptDeviceTy[NumDevices];
+}
+
+void OmptDeviceCallbacksTy::release() { delete[] devices; }
+
+OmptDeviceTy *OmptDeviceCallbacksTy::lookupDevice(int DeviceNum) {
+  return &devices[DeviceNum];
+}
+
 /// Lookup function used for querying callback functions maintained
 /// by the plugin
 ompt_interface_fn_t
 OmptDeviceCallbacksTy::doLookup(const char *InterfaceFunctionName) {
   // TODO This will be populated with device tracing functions
   return (ompt_interface_fn_t) nullptr;
 }
 

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

 #include "DeviceEnvironment.h"
 #include "get_elf_mach_gfx_name.h"
 #include "omptargetplugin.h"
 #include "print_tracing.h"
 
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
 
+#ifdef OMPT_SUPPORT
+#include "ompt_device_callbacks.h"
+#define OMPT_IF_BUILT_AND_ENABLED(stmts)                                       \
+  do {                                                                         \
+    if (ompt_device_callbacks.isEnabled()) {                                   \
+      stmts;                                                                   \
+    }                                                                          \
+  } while (0)
+#else
+#define OMPT_IF_BUILT_AND_ENABLED(stmts)
+#endif
+
 // hostrpc interface, FIXME: consider moving to its own include these are
 // statically linked into amdgpu/plugin if present from hostrpc_services.a,
 // linked as --whole-archive to override the weak symbols that are used to
 // implement a fallback for toolchains that do not yet have a hostrpc library.
 extern "C" {
 uint64_t hostrpc_assign_buffer(hsa_agent_t agent, hsa_queue_t *this_Q,
                                uint32_t device_id);
 hsa_status_t hostrpc_init();
static int readEnv(const char *Env, int Default = -1) {
     int res = Default;
     if (envStr) {
       res = std::stoi(envStr);
       DP("Parsed %s=%d\n", Env, res);
     }
     return res;
   }
 
+#ifdef OMPT_SUPPORT
+  void doOmptDeviceLoad(int32_t DeviceId, const char *FileName,
+                        int64_t OffsetInFile, void *VmaInFile, size_t Bytes,
+                        void *HostAddr, void *DeviceAddr, uint64_t ModuleId) {
+    ompt_device_callbacks.OmptCallbackDeviceLoad(
+        DeviceId, FileName, OffsetInFile, VmaInFile, Bytes, HostAddr,
+        DeviceAddr, ModuleId);
+  }
+
+  void doOmptDeviceInitialize(int32_t DeviceId, const char *InfoName) {
+    std::string OmptGPUType("AMD ");
+    OmptGPUType += InfoName;
+    ompt_device_callbacks.OmptCallbackDeviceInitialize(DeviceId,
+                                                       OmptGPUType.c_str());
+  }
+
+  void doOmptDeviceFinalize() {
+    for (int i = 0; i < NumberOfDevices; i++)
+      ompt_device_callbacks.OmptCallbackDeviceFinalize(i);
+    ompt_device_callbacks.releaseDevices();
+  }
+#endif
+
   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
RTLDeviceInfoTy() {
     }
 
     err = setupHostMemoryPools(CPUAgents);
     if (err != HSA_STATUS_SUCCESS) {
       DP("Setup for Host Memory Pools failed\n");
       return;
     }
 
+#ifdef OMPT_SUPPORT
+    ompt_device_callbacks.prepareDevices(NumberOfDevices);
+#endif
+
     for (int i = 0; i < NumberOfDevices; i++) {
       uint32_t queue_size = 0;
       {
         hsa_status_t err = hsa_agent_get_info(
             HSAAgents[i], HSA_AGENT_INFO_QUEUE_MAX_SIZE, &queue_size);
         if (err != HSA_STATUS_SUCCESS) {
           DP("HSA query QUEUE_MAX_SIZE failed for agent %d\n", i);
           return;
#endif
   }
 
   ~RTLDeviceInfoTy() {
     DP("Finalizing the " GETNAME(TARGET_NAME) " DeviceInfo.\n");
     if (!HSAInitSuccess()) {
       // Then none of these can have been set up and they can't be torn down
       return;
     }
+
+    OMPT_IF_BUILT_AND_ENABLED(doOmptDeviceFinalize());
+
     // Run destructors on types that use HSA before
     // impl_finalize removes access to it
     deviceStateStore.clear();
     KernelArgPoolMap.clear();
     // Terminate hostrpc before finalizing hsa
     hostrpc_terminate();
 
     hsa_status_t Err;
DP("Device %d: default limit for groupsPerDevice %d & threadsPerGroup %d\n",
      DeviceInfo.ThreadsPerGroup[device_id]);
 
   DP("Device %d: wavefront size %d, total threads %d x %d = %d\n", device_id,
      DeviceInfo.WarpSize[device_id], DeviceInfo.ThreadsPerGroup[device_id],
      DeviceInfo.GroupsPerDevice[device_id],
      DeviceInfo.GroupsPerDevice[device_id] *
          DeviceInfo.ThreadsPerGroup[device_id]);
 
+  OMPT_IF_BUILT_AND_ENABLED(
+      DeviceInfo.doOmptDeviceInitialize(device_id, GetInfoName));
+
   return OFFLOAD_SUCCESS;
 }
 
 static __tgt_target_table *
 __tgt_rtl_load_binary_locked(int32_t device_id, __tgt_device_image *image);
 
 __tgt_target_table *__tgt_rtl_load_binary(int32_t device_id,
                                           __tgt_device_image *image) {
if (!elf_machine_id_is_amdgcn(image)) {
     err = env.after_loading();
     if (err != HSA_STATUS_SUCCESS) {
       return NULL;
     }
   }
 
   DP("AMDGPU module successfully loaded!\n");
 
+  OMPT_IF_BUILT_AND_ENABLED(DeviceInfo.doOmptDeviceLoad(
+      device_id, nullptr /* FileName */, 0 /* OffsetInFile */,
+      nullptr /* VmaInFile */, img_size, image->ImageStart,
+      nullptr /* DeviceAddr */, 0 /* FIXME ModuleId */));
   {
     // the device_State array is either large value in bss or a void* that
     // needs to be assigned to a pointer to an array of size device_state_bytes
     // If absent, it has been deadstripped and needs no setup.
 
     void *state_ptr;
     uint32_t state_ptr_size;
     auto &SymbolInfoMap = DeviceInfo.SymbolInfoTable[device_id];

openmp/runtime/src/ompt-event-specific.h

 #define ompt_callback_target_emi_implemented ompt_event_UNIMPLEMENTED
 #define ompt_callback_target_data_op_implemented ompt_event_UNIMPLEMENTED
 #define ompt_callback_target_data_op_emi_implemented ompt_event_UNIMPLEMENTED
 #define ompt_callback_target_submit_implemented ompt_event_UNIMPLEMENTED
 #define ompt_callback_target_submit_emi_implemented ompt_event_UNIMPLEMENTED
 
 #define ompt_callback_control_tool_implemented ompt_event_MAY_ALWAYS
 
-#define ompt_callback_device_initialize_implemented ompt_event_UNIMPLEMENTED
-#define ompt_callback_device_finalize_implemented ompt_event_UNIMPLEMENTED
+#define ompt_callback_device_initialize_implemented ompt_event_MAY_ALWAYS
+#define ompt_callback_device_finalize_implemented ompt_event_MAY_ALWAYS
 
-#define ompt_callback_device_load_implemented ompt_event_UNIMPLEMENTED
+#define ompt_callback_device_load_implemented ompt_event_MAY_ALWAYS
 #define ompt_callback_device_unload_implemented ompt_event_UNIMPLEMENTED
 
 /*----------------------------------------------------------------------------
  | Optional Events
  +--------------------------------------------------------------------------*/
 
 #define ompt_callback_sync_region_wait_implemented                             \
   ompt_event_MAY_ALWAYS_OPTIONAL