[PATCH] D38386: AMDGPU: Add ELFOSABI_AMDGPU_PAL

[Konstantin Zhuravlyov via Phabricator via llvm-commits]

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