[PATCH] D38386: AMDGPU: Add ELFOSABI_AMDGPU_PAL

[Tony Tye via Phabricator via llvm-commits]

t-tye requested changes to this revision.
t-tye added inline comments.
This revision now requires changes to proceed.


================
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],
----------------
kzhuravl wrote:
> 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.
Still think should be testing the range of architecture specific codes, and use the general case if outside the range.


https://reviews.llvm.org/D38386