[Openmp-commits] [PATCH] D103545: [NFC][libomptarget] Reduce the dependency on libelf

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield added a subscriber: pdhaliwal.
JonChesterfield added a comment.

Looks functionally correct. Always in favour of moving stuff out of headers where we can, though as an unrelated change we might want to start building plugins with flto or similar to ensure that remains free.

Either myself or @pdhaliwal will do equivalent handling for the amdgpu plugin.

There was some discussion on the mailing list about failing to link against llvm libs but I didn't see the resolution to it. Suggest we continue as planned.



================
Comment at: openmp/libomptarget/plugins/common/elf_common/elf_common.cpp:22
+#ifndef TARGET_NAME
+#define TARGET_NAME ELF Common
+#endif
----------------
A bit surprised that GETNAME handles spaces. Have you done a debug build of this to check the DP() handling compiles cleanly?


================
Comment at: openmp/libomptarget/plugins/common/elf_common/elf_common.cpp:65
+
+  StringRef StrBuf(ImgBegin, ImgSize);
+  std::unique_ptr<MemoryBuffer> MemBuf =
----------------
Maybe a helper function for this setup code? It's very similar to the above function. Thinking something along the lines of:

ELFObjectFileBase * tbd(char * bytes, size_t size); // return nullptr if it can't make one

but it's possible memory management will thwart that, in which case it may be easier callback style:
```
template <typename F>
int withBytesAsElf(char * bytes, size_t size, F f)
{
 ///
int rc = f(Object);
///
return rc;
}
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103545/new/

https://reviews.llvm.org/D103545