[Openmp-commits] [PATCH] D99553: [libomptarget] Read standard notes for ELF offload images

[Jon Chesterfield via Phabricator via Openmp-commits]

JonChesterfield accepted this revision.
JonChesterfield added a comment.
This revision is now accepted and ready to land.

The commit message is out of date.

The implementation is very clear. Thank you for the ping, missed the patch update email.



================
Comment at: openmp/libomptarget/plugins/common/elf_common/elf_light.cpp:25
+// Helper methods to align addresses.
+template <typename T> inline T alignDown(T value, size_t alignment) {
+  return (T)(value & ~(alignment - 1));
----------------
stuff like this is likely to be available in a freestanding llvm header already


================
Comment at: openmp/libomptarget/plugins/common/elf_common/elf_light.cpp:42
+
+// FIXME: this is taken from openmp/libomptarget/plugins/amdgpu/impl/system.cpp,
+//        but it may be incorrect for 64-bit ELF. Elf64_Nhdr and Elf32_Nhdr
----------------
I believe this to be correct for 64 bit elf. For example, from https://llvm.org/doxygen/BinaryFormat_2ELF_8h_source.html#l01659

```
using Elf64_Word = uint32_t;
// ...
// Node header for ELF64.
 struct Elf64_Nhdr {
   Elf64_Word n_namesz;
   Elf64_Word n_descsz;
   Elf64_Word n_type;
 };
```

I remember being surprised by Elf64_word being uint32_t last time I saw this.


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

https://reviews.llvm.org/D99553