[AMDGPU] add AMDGPU target support to ELFObjectFile.h header

[vpykhtin]

Summary

Allows to test Obj->getArch () == Triple::amdgpu and prints "ELF-amdgpu-hsacobj" on ELF dump.

[arsenm]

include/llvm/Object/ELFObjectFile.h

848 ↗

(On Diff #47656)

Why is the -unknown necessary?

Also should this be amdgcn? The other targets look like they are using the regular arch prefix here

[vpykhtin]

include/llvm/Object/ELFObjectFile.h

848 ↗

(On Diff #47656)

Don't have any preference here, let it be amdgcn.

HSA Code Object spec has requirement :

EF.getHeader()->e_ident[ELF::EI_CLASS] == ELF::EM_AMDGPU
&& EF.getHeader()->e_ident[ELF::EI_OSABI] == ELF::ELFOSABI_AMDGPU_HSA
&& IsLittleEndian

all other I consider "unknown" though not sticking on it either.

[vpykhtin]

per review fixes.

[arsenm]

include/llvm/Object/ELFObjectFile.h

848 ↗

(On Diff #47803)

I would expect the enum name to change. Is this the one standardized somewhere? If we can't change the enum, then I guess the string should match

[vpykhtin]

include/llvm/Object/ELFObjectFile.h

848 ↗

(On Diff #47803)

Well, EM_AMDGPU extensively used and match the name in the HSA Code Object spec so I change name to "ELFxx-amdgpu".

[vpykhtin]

ELFxx-amdgpu name used.

[vpykhtin]

Kind reminder if someone can take a look at this.

[Bigcheese]

Two things. This needs tests, and i want to make sure that we use the same file format name as binutils.

[vpykhtin]

In D17144#364938, @Bigcheese wrote:

Two things. This needs tests, and i want to make sure that we use the same file format name as binutils.

Added tests.

Michael, this is the first place we introducing format name, what would make you sure it will be used in binutils? Should it be registered somewhere?

[• rafael]

So, is this the same target that has been added to lld?

Before we continue adding support for it in llvm I want to make sure the address X offset issue is resolved.

[Bigcheese]

lgtm. This being the only place it's currently being added is good enough.

[• rafael]

Please don't.

What is currently being produced by lld is simply not valid elf. We
should not be spreading it before it is fixed.

Cheers,
Rafael

[ruiu]

I agree with Rafael. Some fields of the AMDGPU ELF file were defined based on a misunderstanding of the ELF file format. Please please fix it before it's too late.

[arsenm]

In D17144#365490, @ruiu wrote:

I agree with Rafael. Some fields of the AMDGPU ELF file were defined based on a misunderstanding of the ELF file format. Please please fix it before it's too late.

What's currently implemented now is a 1st draft of the spec.Soon a newer version will be implemented which I'm pretty sure fixes this issue

[llvm-commits]

Can you fix it and then add code to LLVM? Adding code to LLVM allows people
to start creating object files in the odd "ELF" format. If a fix is not
made in a timely manner, it can be too late, and there's a chance that you
will have no choice other than supporting the old broken format.

• msg-20238-344.txt162 BDownload

[nhaustov]

Can you fix it and then add code to LLVM? Adding code to LLVM allows people
to start creating object files in the odd "ELF" format. If a fix is not
made in a timely manner, it can be too late, and there's a chance that you
will have no choice other than supporting the old broken format.

Is the concern more about custom non-standard code in lld for AMDGPU? We are now actively working on making it more standard.

This change is more about general support in LLVM for EM_AMDGPU and allows us to run llvm-objdump on AMDGPU ELFs to verify changes in disassembler. EM_AMDGPU is officially registered too, see http://www.sco.com/developers/gabi/latest/ch4.eheader.html

Assuming we work on removing non-standard code from lld, is there anything wrong with this change?

[ruiu]

No, there's no particular concern regarding this change itself. I wanted to see it being removed until the AMDGPU support becomes generally available. Because the code for the nonstandard part of the ABI is being removed from lld, I'm fine with this change. Thank you for doing this.

[• rafael]

This is fine by me now that the non-elf bits were removed from lld.

llvm/trunk/include/llvm/Object/ELFObjectFile.h

case ELF::EM_MIPS:
       return "ELF32-mips";
     case ELF::EM_PPC:
       return "ELF32-ppc";
     case ELF::EM_SPARC:
     case ELF::EM_SPARC32PLUS:
       return "ELF32-sparc";
     case ELF::EM_WEBASSEMBLY:
       return "ELF32-wasm";
+    case ELF::EM_AMDGPU:
+      return "ELF32-amdgpu";
     default:
       return "ELF32-unknown";
     }
   case ELF::ELFCLASS64:
     switch (EF.getHeader()->e_machine) {
     case ELF::EM_386:
       return "ELF64-i386";
     case ELF::EM_X86_64:
       return "ELF64-x86-64";
     case ELF::EM_AARCH64:
       return (IsLittleEndian ? "ELF64-aarch64-little" : "ELF64-aarch64-big");
     case ELF::EM_PPC64:
       return "ELF64-ppc64";
     case ELF::EM_S390:
       return "ELF64-s390";
     case ELF::EM_SPARCV9:
       return "ELF64-sparc";
     case ELF::EM_MIPS:
       return "ELF64-mips";
     case ELF::EM_WEBASSEMBLY:
       return "ELF64-wasm";
+    case ELF::EM_AMDGPU:
+      return (EF.getHeader()->e_ident[ELF::EI_OSABI] == ELF::ELFOSABI_AMDGPU_HSA
+              && IsLittleEndian) ?
+             "ELF64-amdgpu-hsacobj" : "ELF64-amdgpu";
     default:
       return "ELF64-unknown";
     }
   default:
     // FIXME: Proper error handling.
     report_fatal_error("Invalid ELFCLASS!");
   }
 }
case ELF::EM_SPARCV9:
     return Triple::sparcv9;
   case ELF::EM_WEBASSEMBLY:
     switch (EF.getHeader()->e_ident[ELF::EI_CLASS]) {
     case ELF::ELFCLASS32: return Triple::wasm32;
     case ELF::ELFCLASS64: return Triple::wasm64;
     default: return Triple::UnknownArch;
     }
 
+  case ELF::EM_AMDGPU:
+    return (EF.getHeader()->e_ident[ELF::EI_CLASS] == ELF::ELFCLASS64
+         && EF.getHeader()->e_ident[ELF::EI_OSABI] == ELF::ELFOSABI_AMDGPU_HSA
+         && IsLittleEndian) ?
+      Triple::amdgcn : Triple::UnknownArch;
+
   default:
     return Triple::UnknownArch;
   }
 }
 
 template <class ELFT>
 ELFObjectFileBase::elf_symbol_iterator_range
 ELFObjectFile<ELFT>::getDynamicSymbolIterators() const {

llvm/trunk/lib/ObjectYAML/ELFYAML.cpp

#define ECase(X) IO.enumCase(Value, #X, ELF::X);
   ECase(ELFOSABI_MODESTO)
   ECase(ELFOSABI_OPENBSD)
   ECase(ELFOSABI_OPENVMS)
   ECase(ELFOSABI_NSK)
   ECase(ELFOSABI_AROS)
   ECase(ELFOSABI_FENIXOS)
   ECase(ELFOSABI_CLOUDABI)
   ECase(ELFOSABI_C6000_ELFABI)
+  ECase(ELFOSABI_AMDGPU_HSA)
   ECase(ELFOSABI_C6000_LINUX)
   ECase(ELFOSABI_ARM)
   ECase(ELFOSABI_STANDALONE)
 #undef ECase
 }
 
 void ScalarBitSetTraits<ELFYAML::ELF_EF>::bitset(IO &IO,
                                                  ELFYAML::ELF_EF &Value) {

llvm/trunk/test/Object/AMDGPU/elf-definitios.yaml

 # RUN: yaml2obj -format=elf %s > %t.o
 # RUN: llvm-readobj -s -file-headers %t.o | FileCheck %s
 
+# CHECK: Format: ELF64-amdgpu-hsacobj
+# CHECK: Arch: amdgcn
 # CHECK:  Machine: EM_AMDGPU (0xE0)
 # CHECK: Sections [
 # CHECK: Section {
 # CHECK: Name: .shf_amdgpu
 # CHECK: Flags [ (0xF00000)
 # CHECK: SHF_AMDGPU_HSA_AGENT (0x800000)
 # CHECK: SHF_AMDGPU_HSA_CODE (0x400000)
 # CHECK: SHF_AMDGPU_HSA_GLOBAL (0x100000)
 # CHECK: SHF_AMDGPU_HSA_READONLY (0x200000)
 # CHECK: }
 
 ---
 FileHeader:
   Class:   ELFCLASS64
   Data:    ELFDATA2LSB
   Type:    ET_REL
   Machine: EM_AMDGPU
+  OSABI:   ELFOSABI_AMDGPU_HSA
 
 Sections:
   - Name:  .shf_amdgpu
     Type:  SHT_PROGBITS
     Flags: [ SHF_AMDGPU_HSA_GLOBAL, SHF_AMDGPU_HSA_READONLY,
              SHF_AMDGPU_HSA_CODE, SHF_AMDGPU_HSA_AGENT]
 ...

llvm/trunk/test/Object/AMDGPU/elf32-unknown.yaml

+# RUN: yaml2obj -format=elf %s > %t.o
+# RUN: llvm-readobj -s -file-headers %t.o | FileCheck %s
+
+# CHECK: Format: ELF32-amdgpu
+
+---
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_AMDGPU

llvm/trunk/test/Object/AMDGPU/elf64-unknown.yaml

+# RUN: yaml2obj -format=elf %s > %t.o
+# RUN: llvm-readobj -s -file-headers %t.o | FileCheck %s
+
+# CHECK: Format: ELF64-amdgpu
+
+---
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_AMDGPU