Skip to content

Commit

Permalink
[Libomptarget][NFCI] Remove caching of created ELF files (#76080)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jhuber6 committed Dec 20, 2023
1 parent b37c048 commit f324584
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.
Expand Down
41 changes: 14 additions & 27 deletions openmp/libomptarget/plugins-nextgen/common/src/GlobalHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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",
Expand All @@ -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;
Expand All @@ -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());
Expand Down
10 changes: 4 additions & 6 deletions openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit f324584

Please sign in to comment.