[PATCH] D143539: [AMDGPU] Add AMDGPU support for llvm-objcopy

Thu Jul 20 10:53:07 PDT 2023

aakanksha555 marked an inline comment as done.
aakanksha555 added a comment.

In D143539#4517371 <https://reviews.llvm.org/D143539#4517371>, @jhenderson wrote:

> Sorry, I was away last week and am still catching up since then.
>
> Is there a reason you haven't just added the new amdgpu case to the existing cross-arch-headers.test file? That seems to me like where it should belong.

Thanks for the feedback.

I ended up putting it in a separate testcase due to how amdgcn arch is handled - https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Object/ELFObjectFile.h#L1327
I'm specifying the relevant flag EF_AMDGPU_MACH_AMDGCN_GFX900 in this standalone test.

================
Comment at: llvm/tools/llvm-objcopy/ObjcopyOptions.cpp:305-306
     // Name, {EMachine, 64bit, LittleEndian}
+    // AMDGPU
+    {"elf64-amdgpu", {ELF::EM_AMDGPU, true, true}},
     // x86
----------------
jhenderson wrote:
> I would suggest that this be moved to the end of the list, purely on the basis that the list has no specific ordering, and newer targets tend to be added at the end.
I'll move it at the bottom of the list

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143539