[Openmp-commits] [openmp] f324584 - [Libomptarget][NFCI] Remove caching of created ELF files (#76080)

[via Openmp-commits]

Author: Joseph Huber
Date: 2023-12-20T17:13:41-06:00
New Revision: f324584ae3b6330be3d79e89cdacd3969760574e

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

LOG: [Libomptarget][NFCI] Remove caching of created ELF files (#76080)

Summary:
We currently keep a cache of created ELF files from the relevant images.
This shouldn't be necessary as the entire ELF interface is generally
trivially constructable and extremely cheap. The cost of constructing
one of these objects is simply a size check and writing a pointer to the
underlying data. Given that, keeping a cache of these images should not
be necessary overall.

Added: 
    

Modified: 
    openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
    openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
    openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h b/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
index fa079ac9660ee0..d9fe938790ca76 100644
--- a/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
+++ b/openmp/libomptarget/plugins-nextgen/common/include/GlobalHandler.h
@@ -89,9 +89,6 @@ template <typename Ty> class StaticGlobalTy : public GlobalTy {
 /// global metadata (size, addr) from the device.
 /// \see getGlobalMetadataFromDevice
 class GenericGlobalHandlerTy {
-  /// Map to store the ELF object files that have been loaded.
-  llvm::DenseMap<int32_t, ELF64LEObjectFile> ELFObjectFiles;
-
   /// Actually move memory between host and device. See readGlobalFromDevice and
   /// writeGlobalToDevice for the interface description.
   Error moveGlobalBetweenDeviceAndHost(GenericDeviceTy &Device,
@@ -109,10 +106,8 @@ class GenericGlobalHandlerTy {
 public:
   virtual ~GenericGlobalHandlerTy() {}
 
-  /// Get the cached ELF64LEObjectFile previosuly created for a specific
-  /// device image or create it if did not exist.
-  const ELF64LEObjectFile *
-  getOrCreateELFObjectFile(const GenericDeviceTy &Device, DeviceImageTy &Image);
+  /// Helper function for getting an ELF from a device image.
+  Expected<ELF64LEObjectFile> getELFObjectFile(DeviceImageTy &Image);
 
   /// Returns whether the symbol named \p SymName is present in the given \p
   /// Image.

diff  --git a/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp b/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
index 3a272e228c7dfe..d398f60c55bd13 100644
--- a/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
@@ -25,29 +25,14 @@ using namespace omp;
 using namespace target;
 using namespace plugin;
 
-const ELF64LEObjectFile *
-GenericGlobalHandlerTy::getOrCreateELFObjectFile(const GenericDeviceTy &Device,
-                                                 DeviceImageTy &Image) {
+Expected<ELF64LEObjectFile>
+GenericGlobalHandlerTy::getELFObjectFile(DeviceImageTy &Image) {
+  assert(utils::elf::isELF(Image.getMemoryBuffer().getBuffer()) &&
+         "Input is not an ELF file");
 
-  auto Search = ELFObjectFiles.find(Image.getId());
-  if (Search != ELFObjectFiles.end())
-    // The ELF object file was already there.
-    return &Search->second;
-
-  // The ELF object file we are checking is not created yet.
   Expected<ELF64LEObjectFile> ElfOrErr =
       ELF64LEObjectFile::create(Image.getMemoryBuffer());
-  if (!ElfOrErr) {
-    consumeError(ElfOrErr.takeError());
-    return nullptr;
-  }
-
-  auto Result =
-      ELFObjectFiles.try_emplace(Image.getId(), std::move(ElfOrErr.get()));
-  assert(Result.second && "Map insertion failed");
-  assert(Result.first != ELFObjectFiles.end() && "Map insertion failed");
-
-  return &Result.first->second;
+  return ElfOrErr;
 }
 
 Error GenericGlobalHandlerTy::moveGlobalBetweenDeviceAndHost(
@@ -83,7 +68,8 @@ Error GenericGlobalHandlerTy::moveGlobalBetweenDeviceAndHost(
       return Err;
   }
 
-  DP("Succesfully %s %u bytes associated with global symbol '%s' %s the device "
+  DP("Succesfully %s %u bytes associated with global symbol '%s' %s the "
+     "device "
      "(%p -> %p).\n",
      Device2Host ? "read" : "write", HostGlobal.getSize(),
      HostGlobal.getName().data(), Device2Host ? "from" : "to",
@@ -98,12 +84,14 @@ bool GenericGlobalHandlerTy::isSymbolInImage(GenericDeviceTy &Device,
   // Get the ELF object file for the image. Notice the ELF object may already
   // be created in previous calls, so we can reuse it. If this is unsuccessful
   // just return false as we couldn't find it.
-  const ELF64LEObjectFile *ELFObj = getOrCreateELFObjectFile(Device, Image);
-  if (!ELFObj)
+  auto ELFObjOrErr = getELFObjectFile(Image);
+  if (!ELFObjOrErr) {
+    consumeError(ELFObjOrErr.takeError());
     return false;
+  }
 
   // Search the ELF symbol using the symbol name.
-  auto SymOrErr = utils::elf::getSymbol(*ELFObj, SymName);
+  auto SymOrErr = utils::elf::getSymbol(*ELFObjOrErr, SymName);
   if (!SymOrErr) {
     consumeError(SymOrErr.takeError());
     return false;
@@ -117,10 +105,9 @@ Error GenericGlobalHandlerTy::getGlobalMetadataFromImage(
 
   // Get the ELF object file for the image. Notice the ELF object may already
   // be created in previous calls, so we can reuse it.
-  const ELF64LEObjectFile *ELFObj = getOrCreateELFObjectFile(Device, Image);
+  auto ELFObj = getELFObjectFile(Image);
   if (!ELFObj)
-    return Plugin::error("Unable to create ELF object for image %p",
-                         Image.getStart());
+    return ELFObj.takeError();
 
   // Search the ELF symbol using the symbol name.
   auto SymOrErr = utils::elf::getSymbol(*ELFObj, ImageGlobal.getName());

diff  --git a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
index 0c7535a0da8b92..b0dff917dd0be0 100644
--- a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
@@ -1063,15 +1063,13 @@ struct CUDADeviceTy : public GenericDeviceTy {
     // automatically so we must create it ourselves. The backend will emit
     // several globals that contain function pointers we can call. These are
     // prefixed with a known name due to Nvidia's lack of section support.
-    const ELF64LEObjectFile *ELFObj =
-        Handler.getOrCreateELFObjectFile(*this, Image);
-    if (!ELFObj)
-      return Plugin::error("Unable to create ELF object for image %p",
-                           Image.getStart());
+    auto ELFObjOrErr = Handler.getELFObjectFile(Image);
+    if (!ELFObjOrErr)
+      return ELFObjOrErr.takeError();
 
     // Search for all symbols that contain a constructor or destructor.
     SmallVector<std::pair<StringRef, uint16_t>> Funcs;
-    for (ELFSymbolRef Sym : ELFObj->symbols()) {
+    for (ELFSymbolRef Sym : ELFObjOrErr->symbols()) {
       auto NameOrErr = Sym.getName();
       if (!NameOrErr)
         return NameOrErr.takeError();