[libomptarget] Load images in order of registration

[manorom]

Summary

This makes sure that images are loaded in the order in which they are registered with libomptarget.

If a target can load multiple images and these images depend on each other (for example if one image contains the programs target regions and one image contains library code), then the order in which images are loaded can be important for symbol resolution (for example, in the VE plugin).
In this case: because the same code exist in the host binaries, the order in which the host linker loads them (which is also the order in which images are registered with libomptarget) is the order in which the images have to be loaded onto the device.

[simoll]

openmp/libomptarget/src/rtl.cpp

151–155

Can we make this a callback instead? Eg,

*((void **)&R.supports_empty_images) =
     dlsym(dynlib_handle, "__tgt_rtl_supports_empty_images");

[manorom]

• Use callback to test for support for empty images

[JonChesterfield]

There are two changes here. Loading images in a given order and loading images even if they look empty.

I'd personally split this, because annoying though it is to split a change, the whole thing is likely to be reverted if part of it turns out to be faulty.

Should fix the formatting nit above before landing. git-clang-format is great for that.

openmp/libomptarget/src/rtl.h

57

This is weird. Most of these typedefs are for function types, as opposed to the corresponding function pointer type. I guess matching the existing ones is right, but I think we're missing a '*' in most of these.

[tianshilei1992]

openmp/libomptarget/src/rtl.h

57

It starts from register_lib_ty and unregister_lib. :-)

[manorom]

• Fix formatting

[tcramer]

I'd personally split this, because annoying though it is to split a change, the whole thing is likely to be reverted if part of it turns out to be faulty.

Since you already accepted this patch, I guess it is ok if we leave it in one patch? If the whole thing has to be reverted it is ok for us. In that case we would fix it with an additonal / splitted patch.

[JonChesterfield]

Yep, fine to commit as one patch. Recommendation was to reduce the amount of rework if this breaks something and gets reverted, not a requirement.

openmp/libomptarget/include/omptargetplugin.h

 // having to load the library, which can be expensive.
 int32_t __tgt_rtl_is_valid_binary(__tgt_device_image *Image);
 
 // Return an integer other than zero if the data can be exchaned from SrcDevId
 // to DstDevId. If it is data exchangable, the device plugin should provide
 // function to move data from source device to destination device directly.
 int32_t __tgt_rtl_is_data_exchangable(int32_t SrcDevId, int32_t DstDevId);
 
+// Return an integer other than zero if the plugin can handle images which do
+// not contain target regions and global variables (but can contain other
+// functions)
+int32_t __tgt_rtl_supports_empty_images();
+
 // Initialize the requires flags for the device.
 int64_t __tgt_rtl_init_requires(int64_t RequiresFlags);
 
 // Initialize the specified device. In case of success return 0; otherwise
 // return an error code.
 int32_t __tgt_rtl_init_device(int32_t ID);
 
 // Pass an executable image section described by image to the specified

openmp/libomptarget/plugins/exports

global:
     __tgt_rtl_data_delete;
     __tgt_rtl_run_target_team_region;
     __tgt_rtl_run_target_team_region_async;
     __tgt_rtl_run_target_region;
     __tgt_rtl_run_target_region_async;
     __tgt_rtl_synchronize;
     __tgt_rtl_register_lib;
     __tgt_rtl_unregister_lib;
+    __tgt_rtl_supports_empty_images;
   local:
     *;
 };

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

 // offsets. An offset should be added to the target address before passing it
 // to the outlined function on device side. In case of success, return zero.
 // Otherwise, return an error code.
 int32_t __tgt_rtl_run_target_region(int32_t ID, void *Entry, void **Args,
                                     ptrdiff_t *Offsets, int32_t NumArgs) {
   return __tgt_rtl_run_target_team_region(ID, Entry, Args, Offsets, NumArgs, 1,
                                           1, 0);
 }
+
+int32_t __tgt_rtl_supports_empty_images() { return 1; }

openmp/libomptarget/src/device.h

struct PluginManager {
 
   /// Devices associated with RTLs
   DevicesTy Devices;
   std::mutex RTLsMtx; ///< For RTLs and Devices
 
   /// Translation table retreived from the binary
   HostEntriesBeginToTransTableTy HostEntriesBeginToTransTable;
   std::mutex TrlTblMtx; ///< For Translation Table
+  /// Host offload entries in order of image registration
+  std::vector<__tgt_offload_entry *> HostEntriesBeginRegistrationOrder;
 
   /// Map from ptrs on the host to an entry in the Translation Table
   HostPtrToTableMapTy HostPtrToTableMap;
   std::mutex TblMapMtx; ///< For HostPtrToTableMap
 
   // Store target policy (disabled, mandatory, default)
   kmp_target_offload_kind_t TargetOffloadPolicy = tgt_default;
   std::mutex TargetOffloadMtx; ///< For TargetOffloadPolicy
 };
 
 extern PluginManager *PM;
 
 #endif

openmp/libomptarget/src/omptarget.cpp

 
 /// Map global data and execute pending ctors
 static int InitLibrary(DeviceTy &Device) {
   /*
    * Map global data
    */
   int32_t device_id = Device.DeviceID;
   int rc = OFFLOAD_SUCCESS;
+  bool supportsEmptyImages = Device.RTL->supports_empty_images &&
+                             Device.RTL->supports_empty_images() > 0;
 
   Device.PendingGlobalsMtx.lock();
   PM->TrlTblMtx.lock();
-  for (HostEntriesBeginToTransTableTy::iterator entry_it =
-           PM->HostEntriesBeginToTransTable.begin();
-       entry_it != PM->HostEntriesBeginToTransTable.end(); ++entry_it) {
-    TranslationTable *TransTable = &entry_it->second;
+  for (auto *HostEntriesBegin : PM->HostEntriesBeginRegistrationOrder) {
+    TranslationTable *TransTable =
+        &PM->HostEntriesBeginToTransTable[HostEntriesBegin];
     if (TransTable->HostTable.EntriesBegin ==
-        TransTable->HostTable.EntriesEnd) {
+            TransTable->HostTable.EntriesEnd &&
+        !supportsEmptyImages) {
       // No host entry so no need to proceed
       continue;
     }
+
     if (TransTable->TargetsTable[device_id] != 0) {
       // Library entries have already been processed
       continue;
     }
 
     // 1) get image.
     assert(TransTable->TargetsImages.size() > (size_t)device_id &&
            "Not expecting a device ID outside the table's bounds!");

openmp/libomptarget/src/rtl.h

typedef int32_t(run_team_region_ty)(int32_t, void *, void **, ptrdiff_t *,
                                       int32_t, int32_t, int32_t, uint64_t);
   typedef int32_t(run_team_region_async_ty)(int32_t, void *, void **,
                                             ptrdiff_t *, int32_t, int32_t,
                                             int32_t, uint64_t,
                                             __tgt_async_info *);
   typedef int64_t(init_requires_ty)(int64_t);
   typedef int64_t(synchronize_ty)(int32_t, __tgt_async_info *);
   typedef int32_t (*register_lib_ty)(__tgt_bin_desc *);
+  typedef int32_t(supports_empty_images_ty)();
 
   int32_t Idx = -1;             // RTL index, index is the number of devices
                                 // of other RTLs that were registered before,
                                 // i.e. the OpenMP index of the first device
                                 // to be registered with this RTL.
   int32_t NumberOfDevices = -1; // Number of devices this RTL deals with.
 
   void *LibraryHandler = nullptr;
#endif
   run_region_ty *run_region = nullptr;
   run_region_async_ty *run_region_async = nullptr;
   run_team_region_ty *run_team_region = nullptr;
   run_team_region_async_ty *run_team_region_async = nullptr;
   init_requires_ty *init_requires = nullptr;
   synchronize_ty *synchronize = nullptr;
   register_lib_ty register_lib = nullptr;
   register_lib_ty unregister_lib = nullptr;
+  supports_empty_images_ty *supports_empty_images = nullptr;
 
   // Are there images associated with this RTL.
   bool isUsed = false;
 
   // Mutex for thread-safety when calling RTL interface functions.
   // It is easier to enforce thread-safety at the libomptarget level,
   // so that developers of new RTLs do not have to worry about it.
   std::mutex Mtx;

openmp/libomptarget/src/rtl.cpp

for (auto *Name : RTLNames) {
     }
 
     R.LibraryHandler = dynlib_handle;
 
 #ifdef OMPTARGET_DEBUG
     R.RTLName = Name;
 #endif
 
     DP("Registering RTL %s supporting %d devices!\n", R.RTLName.c_str(),
        R.NumberOfDevices);
 
     // Optional functions
     *((void **)&R.init_requires) =
         dlsym(dynlib_handle, "__tgt_rtl_init_requires");
     *((void **)&R.data_submit_async) =
         dlsym(dynlib_handle, "__tgt_rtl_data_submit_async");
     *((void **)&R.data_retrieve_async) =
         dlsym(dynlib_handle, "__tgt_rtl_data_retrieve_async");
     *((void **)&R.run_region_async) =
         dlsym(dynlib_handle, "__tgt_rtl_run_target_region_async");
     *((void **)&R.run_team_region_async) =
         dlsym(dynlib_handle, "__tgt_rtl_run_target_team_region_async");
     *((void **)&R.synchronize) = dlsym(dynlib_handle, "__tgt_rtl_synchronize");
     *((void **)&R.data_exchange) =
         dlsym(dynlib_handle, "__tgt_rtl_data_exchange");
     *((void **)&R.data_exchange_async) =
         dlsym(dynlib_handle, "__tgt_rtl_data_exchange_async");
     *((void **)&R.is_data_exchangable) =
         dlsym(dynlib_handle, "__tgt_rtl_is_data_exchangable");
     *((void **)&R.register_lib) =
         dlsym(dynlib_handle, "__tgt_rtl_register_lib");
     *((void **)&R.unregister_lib) =
         dlsym(dynlib_handle, "__tgt_rtl_unregister_lib");
+    *((void **)&R.supports_empty_images) =
+        dlsym(dynlib_handle, "__tgt_rtl_supports_empty_images");
   }
 
   DP("RTLs loaded!\n");
 
   return;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
for (auto &R : AllRTLs) {
         UsedRTLs.push_back(&R);
 
         DP("RTL " DPxMOD " has index %d!\n", DPxPTR(R.LibraryHandler), R.Idx);
       }
 
       // Initialize (if necessary) translation table for this library.
       PM->TrlTblMtx.lock();
       if (!PM->HostEntriesBeginToTransTable.count(desc->HostEntriesBegin)) {
+        PM->HostEntriesBeginRegistrationOrder.push_back(desc->HostEntriesBegin);
         TranslationTable &TransTable =
             (PM->HostEntriesBeginToTransTable)[desc->HostEntriesBegin];
         TransTable.HostTable.EntriesBegin = desc->HostEntriesBegin;
         TransTable.HostTable.EntriesEnd = desc->HostEntriesEnd;
       }
 
       // Retrieve translation table for this library.
       TranslationTable &TransTable =