[PATCH] D38386: AMDGPU: Add ELFOSABI_AMDGPU_PAL

Konstantin Zhuravlyov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 29 10:56:31 PDT 2017


kzhuravl added inline comments.


================
Comment at: tools/llvm-readobj/ELFDumper.cpp:3521
       // Handle architecture specific OS/ABI values.
-      if (e->e_machine == ELF::EM_AMDGPU &&
-          e->e_ident[ELF::EI_OSABI] == ELF::ELFOSABI_AMDGPU_HSA)
-        W.printHex("OS/ABI", "AMDGPU_HSA", ELF::ELFOSABI_AMDGPU_HSA);
+      if (e->e_machine == ELF::EM_AMDGPU)
+        W.printEnum("OS/ABI", e->e_ident[ELF::EI_OSABI],
----------------
t-tye wrote:
> Should this be:
> 
> 
> ```
> if (e->e_machine == ELF::EM_AMDGPU && (e->e_ident[ELF::EI_OSABI] >= ELF::EI_FIRST_ARCH && e->e_ident[ELF::EI_OSABI] <= ELF::EI_LAST_ARCH))
> ```
> 
> Since only this range is the architecture specific, and other values should be handled by the ElfOSABI table. Technically there are some entries in that table that are architecture specific and probably ought to be handled in a similar way so that different architectures can use the same values, but if desired that can be done as another patch.
Yes, I was planning to do it in separate patch.


https://reviews.llvm.org/D38386





More information about the llvm-commits mailing list