[Openmp-commits] [openmp] [Libomptarget][NFCI] Remove caching of created ELF files (PR #76080)
Joseph Huber via Openmp-commits
openmp-commits at lists.llvm.org
Wed Dec 20 09:21:36 PST 2023
https://github.com/jhuber6 created https://github.com/llvm/llvm-project/pull/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.
>From 960d1d7cab9cc5f6d989211d5cb1de777cfee9af Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Wed, 20 Dec 2023 11:19:01 -0600
Subject: [PATCH] [Libomptarget][NFCI] Remove caching of created ELF files
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.
---
.../common/include/GlobalHandler.h | 9 +---
.../common/src/GlobalHandler.cpp | 41 +++++++------------
.../plugins-nextgen/cuda/src/rtl.cpp | 10 ++---
3 files changed, 20 insertions(+), 40 deletions(-)
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();
More information about the Openmp-commits
mailing list