[Openmp-commits] [openmp] [OpenMP][NFC] Merge elf_common into PluginInterface (PR #73677)

via Openmp-commits openmp-commits at lists.llvm.org
Tue Nov 28 09:53:48 PST 2023


llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-openmp

Author: Johannes Doerfert (jdoerfert)

<details>
<summary>Changes</summary>

The 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:

- (modified) openmp/libomptarget/plugins-nextgen/CMakeLists.txt (-1) 
- (modified) openmp/libomptarget/plugins-nextgen/amdgpu/CMakeLists.txt (-1) 
- (modified) openmp/libomptarget/plugins-nextgen/common/CMakeLists.txt (-1) 
- (modified) openmp/libomptarget/plugins-nextgen/common/OMPT/CMakeLists.txt (-1) 
- (modified) openmp/libomptarget/plugins-nextgen/common/PluginInterface/CMakeLists.txt (+6-2) 
- (modified) openmp/libomptarget/plugins-nextgen/common/PluginInterface/GlobalHandler.cpp (+2-2) 
- (modified) openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp (+2-2) 
- (renamed) openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.cpp (+64-4) 
- (renamed) openmp/libomptarget/plugins-nextgen/common/PluginInterface/Utils/ELF.h (+19-7) 
- (removed) openmp/libomptarget/plugins-nextgen/common/elf_common/CMakeLists.txt (-36) 
- (removed) openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.cpp (-88) 
- (removed) openmp/libomptarget/plugins-nextgen/common/elf_common/elf_common.h (-27) 
- (modified) openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt (-1) 


``````````diff
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}

``````````

</details>


https://github.com/llvm/llvm-project/pull/73677


More information about the Openmp-commits mailing list