[OpenMP][NFC] Merge elf_common into PluginInterface #73677
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The overhead of a library and 4 files seems high without benefit. This simply tries to consolidate our structure.
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-openmp Author: Johannes Doerfert (jdoerfert) ChangesThe overhead of a library and 4 files seems high without benefit. This simply tries to consolidate our structure. Full diff: https://github.com/llvm/llvm-project/pull/73677.diff 13 Files Affected:
diff --git a/openmp/libomptarget/plugins-nextgen/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
index 5f97a77327c0fef..966f51621c322d5 100644
--- a/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/CMakeLists.txt
@@ -47,7 +47,6 @@ if(CMAKE_SYSTEM_PROCESSOR MATCHES "${tmachine}$")
LINK_LIBS
PRIVATE
- elf_common
MemoryManager
PluginInterface
${LIBOMPTARGET_DEP_LIBFFI_LIBRARIES}
diff --git a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
index eac1ebe4fb52b39..94a0084db6077ad 100644
--- a/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt
@@ -80,7 +80,6 @@ add_llvm_library(omptarget.rtl.amdgpu SHARED
LINK_LIBS
PRIVATE
- elf_common
MemoryManager
PluginInterface
${LIBOMPTARGET_DEP_LIBRARIES}
diff --git a/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
index 368f70984b0bdea..3d1f2634e6bc496 100644
--- a/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt
@@ -13,4 +13,3 @@
add_subdirectory(OMPT)
add_subdirectory(PluginInterface)
add_subdirectory(MemoryManager)
-add_subdirectory(elf_common)
diff --git a/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
index c50dd18dce023b4..db79c6d3e550ee5 100644
--- a/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt
@@ -52,7 +52,6 @@ endif()
target_link_libraries(OMPT
PUBLIC
${llvm_libs}
- elf_common
MemoryManager
)
diff --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
index deecda2732bb529..4ff4dee3c0875f7 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt
@@ -13,7 +13,12 @@
# NOTE: Don't try to build `PluginInterface` using `add_llvm_library` because we
# don't want to export `PluginInterface` while `add_llvm_library` requires that.
add_library(PluginInterface OBJECT
- PluginInterface.cpp GlobalHandler.cpp JIT.cpp RPC.cpp)
+ PluginInterface.cpp
+ GlobalHandler.cpp
+ JIT.cpp
+ RPC.cpp
+ Utils/ELF.cpp
+)
# Only enable JIT for those targets that LLVM can support.
string(TOUPPER "${LLVM_TARGETS_TO_BUILD}" TargetsSupported)
@@ -58,7 +63,6 @@ endif()
target_link_libraries(PluginInterface
PUBLIC
${llvm_libs}
- elf_common
MemoryManager
)
diff --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.cpp b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.cpp
index b375d77f2023b02..9cb1da371fa9ac4 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.cpp
@@ -11,9 +11,9 @@
//===----------------------------------------------------------------------===//
#include "GlobalHandler.h"
-#include "ELFSymbols.h"
#include "PluginInterface.h"
#include "Utilities.h"
+#include "Utils/ELF.h"
#include <cstring>
@@ -115,7 +115,7 @@ Error GenericGlobalHandlerTy::getGlobalMetadataFromImage(
Image.getStart());
// Search the ELF symbol using the symbol name.
- auto SymOrErr = getELFSymbol(*ELFObj, ImageGlobal.getName());
+ auto SymOrErr = utils::elf::getSymbol(*ELFObj, ImageGlobal.getName());
if (!SymOrErr)
return Plugin::error("Failed ELF lookup of global '%s': %s",
ImageGlobal.getName().data(),
diff --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
index 39acf9699931947..2ee79d3131ff3df 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -13,7 +13,7 @@
#include "Environment.h"
#include "GlobalHandler.h"
#include "JIT.h"
-#include "elf_common.h"
+#include "Utils/ELF.h"
#include "omptarget.h"
#include "omptargetplugin.h"
@@ -1636,7 +1636,7 @@ int32_t __tgt_rtl_is_valid_binary(__tgt_device_image *TgtImage) {
if (!Plugin::isActive())
return false;
- if (elf_check_machine(TgtImage, Plugin::get().getMagicElfBits()))
+ if (utils::elf::checkMachine(TgtImage, Plugin::get().getMagicElfBits()))
return true;
return Plugin::get().getJIT().checkBitcodeImage(*TgtImage);
diff --git a/openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.cpp b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.cpp
similarity index 77%
rename from openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.cpp
rename to openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.cpp
index 36ed9aef3a7e4f2..e0c46e4a288e926 100644
--- a/openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.cpp
@@ -1,16 +1,76 @@
-//===-- ELFSymbols.cpp - ELF Symbol look-up functionality -------*- C++ -*-===//
+//===-- Utils/ELF.cpp - Common ELF functionality --------------------------===//
//
// 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
//
//===----------------------------------------------------------------------===//
+//
+// Common ELF functionality for target plugins.
+//
+//===----------------------------------------------------------------------===//
-#include "ELFSymbols.h"
+#include "ELF.h"
+#include "Debug.h"
+
+#include "llvm/BinaryFormat/Magic.h"
+#include "llvm/Object/Binary.h"
+#include "llvm/Object/ELFObjectFile.h"
+#include "llvm/Object/ELFTypes.h"
+#include "llvm/Object/ObjectFile.h"
+#include "llvm/Support/MemoryBuffer.h"
using namespace llvm;
-using namespace llvm::object;
using namespace llvm::ELF;
+using namespace llvm::object;
+
+/// If the given range of bytes [\p BytesBegin, \p BytesEnd) represents
+/// a valid ELF, then invoke \p Callback on the ELFObjectFileBase
+/// created from this range, otherwise, return 0.
+/// If \p Callback is invoked, then return whatever value \p Callback returns.
+template <typename F>
+static int32_t withBytesAsElf(char *BytesBegin, char *BytesEnd, F Callback) {
+ size_t Size = BytesEnd - BytesBegin;
+ StringRef StrBuf(BytesBegin, Size);
+
+ auto Magic = identify_magic(StrBuf);
+ if (Magic != file_magic::elf && Magic != file_magic::elf_relocatable &&
+ Magic != file_magic::elf_executable &&
+ Magic != file_magic::elf_shared_object && Magic != file_magic::elf_core) {
+ DP("Not an ELF image!\n");
+ return 0;
+ }
+
+ std::unique_ptr<MemoryBuffer> MemBuf =
+ MemoryBuffer::getMemBuffer(StrBuf, "", false);
+ Expected<std::unique_ptr<ObjectFile>> BinOrErr =
+ ObjectFile::createELFObjectFile(MemBuf->getMemBufferRef(),
+ /*InitContent=*/false);
+ if (!BinOrErr) {
+ DP("Unable to get ELF handle: %s!\n",
+ toString(BinOrErr.takeError()).c_str());
+ return 0;
+ }
+
+ auto *Object = dyn_cast<const ELFObjectFileBase>(BinOrErr->get());
+
+ if (!Object) {
+ DP("Unknown ELF format!\n");
+ return 0;
+ }
+
+ return Callback(Object);
+}
+
+// Check whether an image is valid for execution on target_id
+int32_t utils::elf::checkMachine(__tgt_device_image *Image, uint16_t TargetId) {
+ auto CheckMachine = [TargetId](const ELFObjectFileBase *Object) {
+ return TargetId == Object->getEMachine();
+ };
+ return withBytesAsElf(reinterpret_cast<char *>(Image->ImageStart),
+ reinterpret_cast<char *>(Image->ImageEnd),
+ CheckMachine);
+}
template <class ELFT>
static Expected<const typename ELFT::Sym *>
@@ -173,7 +233,7 @@ getSymTableSymbol(const ELFFile<ELFT> &Elf, const typename ELFT::Shdr &Sec,
}
Expected<const typename ELF64LE::Sym *>
-getELFSymbol(const ELFObjectFile<ELF64LE> &ELFObj, StringRef Name) {
+utils::elf::getSymbol(const ELFObjectFile<ELF64LE> &ELFObj, StringRef Name) {
// First try to look up the symbol via the hash table.
for (ELFSectionRef Sec : ELFObj.sections()) {
if (Sec.getType() != SHT_HASH && Sec.getType() != SHT_GNU_HASH)
diff --git a/openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.h b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.h
similarity index 53%
rename from openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.h
rename to openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.h
index 589cd3436c3266e..6b1ce434f0c4e18 100644
--- a/openmp/libomptarget/plugins-nextgen/common/elf_common/ELFSymbols.h
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.h
@@ -1,4 +1,4 @@
-//===-- ELFSymbols.h - ELF Symbol look-up functionality ---------*- C++ -*-===//
+//===-- Utils/ELF.h - Common ELF functionality ------------------*- C++ -*-===//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
@@ -6,22 +6,34 @@
//
//===----------------------------------------------------------------------===//
//
-// ELF routines for obtaining a symbol from an Elf file without loading it.
+// Common ELF functionality for target plugins.
//
//===----------------------------------------------------------------------===//
-#ifndef LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_ELF_COMMON_ELF_SYMBOLS_H
-#define LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_ELF_COMMON_ELF_SYMBOLS_H
+#ifndef LLVM_OPENMP_LIBOMPTARGET_PLUGINS_ELF_UTILS_H
+#define LLVM_OPENMP_LIBOMPTARGET_PLUGINS_ELF_UTILS_H
+
+#include "omptargetplugin.h"
#include "llvm/Object/ELF.h"
#include "llvm/Object/ELFObjectFile.h"
+namespace utils {
+namespace elf {
+
+/// Return non-zero, if the given \p image is an ELF object, which
+/// e_machine matches \p target_id; return zero otherwise.
+EXTERN int32_t checkMachine(__tgt_device_image *Image, uint16_t TargetId);
+
/// Returns the symbol associated with the \p Name in the \p ELFObj. It will
/// first search for the hash sections to identify symbols from the hash table.
/// If that fails it will fall back to a linear search in the case of an
/// executable file without a hash table.
llvm::Expected<const typename llvm::object::ELF64LE::Sym *>
-getELFSymbol(const llvm::object::ELFObjectFile<llvm::object::ELF64LE> &ELFObj,
- llvm::StringRef Name);
+getSymbol(const llvm::object::ELFObjectFile<llvm::object::ELF64LE> &ELFObj,
+ llvm::StringRef Name);
+
+} // namespace elf
+} // namespace utils
-#endif
+#endif // LLVM_OPENMP_LIBOMPTARGET_PLUGINS_ELF_UTILS_H
diff --git a/openmp/libomptarget/plugins-nextgen/common/elf_common/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/common/elf_common/CMakeLists.txt
deleted file mode 100644
index 4d3a4019f1dd5dc..000000000000000
--- a/openmp/libomptarget/plugins-nextgen/common/elf_common/CMakeLists.txt
+++ /dev/null
@@ -1,36 +0,0 @@
-##===----------------------------------------------------------------------===##
-#
-# 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
-#
-##===----------------------------------------------------------------------===##
-#
-# Common ELF functionality for target plugins
-#
-##===----------------------------------------------------------------------===##
-
-# NOTE: Don't try to build `elf_common` using `add_llvm_library`.
-# See openmp/libomptarget/plugins/common/PluginInterface/CMakeLists.txt
-# for more explanation.
-add_library(elf_common OBJECT elf_common.cpp ELFSymbols.cpp)
-
-# This is required when using LLVM libraries.
-llvm_update_compile_flags(elf_common)
-
-if (LLVM_LINK_LLVM_DYLIB)
- set(llvm_libs LLVM)
-else()
- llvm_map_components_to_libnames(llvm_libs BinaryFormat Object Support)
-endif()
-
-target_link_libraries(elf_common PUBLIC ${llvm_libs} ${OPENMP_PTHREAD_LIB})
-
-# Build elf_common with PIC to be able to link it with plugin shared libraries.
-set_property(TARGET elf_common PROPERTY POSITION_INDEPENDENT_CODE ON)
-
-# Expose elf_common.h directory to the users of this library.
-target_include_directories(elf_common
- INTERFACE ${CMAKE_CURRENT_SOURCE_DIR}
- PRIVATE ${LIBOMPTARGET_INCLUDE_DIR}
-)
diff --git a/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.cpp b/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.cpp
deleted file mode 100644
index 43ad4477c0cabf5..000000000000000
--- a/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.cpp
+++ /dev/null
@@ -1,88 +0,0 @@
-//===-- elf_common.cpp - Common ELF functionality -------------------------===//
-//
-// 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
-//
-//===----------------------------------------------------------------------===//
-//
-// Common ELF functionality for target plugins.
-//
-//===----------------------------------------------------------------------===//
-#include "elf_common.h"
-#include "Debug.h"
-
-#include "llvm/BinaryFormat/Magic.h"
-#include "llvm/Object/Binary.h"
-#include "llvm/Object/ELFObjectFile.h"
-#include "llvm/Object/ELFTypes.h"
-#include "llvm/Object/ObjectFile.h"
-#include "llvm/Support/MemoryBuffer.h"
-
-#ifndef TARGET_NAME
-#define TARGET_NAME ELF Common
-#endif
-#define DEBUG_PREFIX "TARGET " GETNAME(TARGET_NAME)
-
-using namespace llvm;
-using namespace llvm::ELF;
-using namespace llvm::object;
-
-/// If the given range of bytes [\p BytesBegin, \p BytesEnd) represents
-/// a valid ELF, then invoke \p Callback on the ELFObjectFileBase
-/// created from this range, otherwise, return 0.
-/// If \p Callback is invoked, then return whatever value \p Callback returns.
-template <typename F>
-static int32_t withBytesAsElf(char *BytesBegin, char *BytesEnd, F Callback) {
- size_t Size = BytesEnd - BytesBegin;
- StringRef StrBuf(BytesBegin, Size);
-
- auto Magic = identify_magic(StrBuf);
- if (Magic != file_magic::elf && Magic != file_magic::elf_relocatable &&
- Magic != file_magic::elf_executable &&
- Magic != file_magic::elf_shared_object && Magic != file_magic::elf_core) {
- DP("Not an ELF image!\n");
- return 0;
- }
-
- std::unique_ptr<MemoryBuffer> MemBuf =
- MemoryBuffer::getMemBuffer(StrBuf, "", false);
- Expected<std::unique_ptr<ObjectFile>> BinOrErr =
- ObjectFile::createELFObjectFile(MemBuf->getMemBufferRef(),
- /*InitContent=*/false);
- if (!BinOrErr) {
- DP("Unable to get ELF handle: %s!\n",
- toString(BinOrErr.takeError()).c_str());
- return 0;
- }
-
- auto *Object = dyn_cast<const ELFObjectFileBase>(BinOrErr->get());
-
- if (!Object) {
- DP("Unknown ELF format!\n");
- return 0;
- }
-
- return Callback(Object);
-}
-
-// Check whether an image is valid for execution on target_id
-int32_t elf_check_machine(__tgt_device_image *Image, uint16_t TargetId) {
- auto CheckMachine = [TargetId](const ELFObjectFileBase *Object) {
- return TargetId == Object->getEMachine();
- };
- return withBytesAsElf(reinterpret_cast<char *>(Image->ImageStart),
- reinterpret_cast<char *>(Image->ImageEnd),
- CheckMachine);
-}
-
-int32_t elf_is_dynamic(__tgt_device_image *Image) {
- auto CheckDynType = [](const ELFObjectFileBase *Object) {
- uint16_t Type = Object->getEType();
- DP("ELF Type: %d\n", Type);
- return Type == ET_DYN;
- };
- return withBytesAsElf(reinterpret_cast<char *>(Image->ImageStart),
- reinterpret_cast<char *>(Image->ImageEnd),
- CheckDynType);
-}
diff --git a/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.h b/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.h
deleted file mode 100644
index 933665e59a45569..000000000000000
--- a/openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.h
+++ /dev/null
@@ -1,27 +0,0 @@
-//===-- elf_common.h - Common ELF functionality -----------------*- 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
-//
-//===----------------------------------------------------------------------===//
-//
-// Common ELF functionality for target plugins.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_ELF_COMMON_ELF_COMMON_H
-#define LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_ELF_COMMON_ELF_COMMON_H
-
-#include "omptargetplugin.h"
-#include <cstdint>
-
-/// Return non-zero, if the given \p image is an ELF object, which
-/// e_machine matches \p target_id; return zero otherwise.
-EXTERN int32_t elf_check_machine(__tgt_device_image *Image, uint16_t TargetId);
-
-/// Return non-zero, if the given \p image is an ET_DYN ELF object;
-/// return zero otherwise.
-EXTERN int32_t elf_is_dynamic(__tgt_device_image *Image);
-
-#endif // LLVM_OPENMP_LIBOMPTARGET_PLUGINS_COMMON_ELF_COMMON_ELF_COMMON_H
diff --git a/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt b/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
index 26022b1bc4d61e2..d0e6b26888d5532 100644
--- a/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
+++ b/openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
@@ -34,7 +34,6 @@ add_llvm_library(omptarget.rtl.cuda SHARED
Object
LINK_LIBS PRIVATE
- elf_common
MemoryManager
PluginInterface
${OPENMP_PTHREAD_LIB}
|
jdoerfert
added a commit
to jdoerfert/llvm-project
that referenced
this pull request
Nov 28, 2023
Similar to llvm#73677, there is no benefit from keeping MemoryManager seperate; it's tied into the current design. Except the move I also replaced the getenv call with our Env handling.
jdoerfert
added a commit
that referenced
this pull request
Nov 28, 2023
Similar to #73677, there is no benefit from keeping MemoryManager seperate; it's tied into the current design. Except the move I also replaced the getenv call with our Env handling.
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Nov 29, 2023
Brings in: [OpenMP][NFC] Merge elf_common into PluginInterface (llvm#73677) Change-Id: If5365287d04449d8a478423c70a8bfb56aee6ac3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The overhead of a library and 4 files seems high without benefit. This simply tries to consolidate our structure.