LLD/ELF/AMDGPU: Process AMDGPU-specific e_flags

[t-tye]

ELF/Arch/AMDGPU.cpp

47

Is it worth avoiding checking the first element since it has already been inspected. For example:

for (const auto &F : ArrayRef(ObjectFiles).slice(1)) {

[ruiu]

ELF/Arch/AMDGPU.cpp

47

Please add a comment saying that this loop is to verify that all input files have the same e_flags.

const auto -> InputFile * as we do not use auto unless its type is apparent.

FirstEFlags -> Ret because we usually prefer short variable names.

48

Can you just remove this temporary variable and directly use the value?

[kzhuravl]

Address review feedback.

[ruiu]

How did you create Inputs/amdgpu-kernel-2.o? We generally want to avoid checking in binary files, so if it is created by llvm-mc or something, we'd like to check in the source file instead of the binary file.

[kzhuravl]

In D39140#905422, @ruiu wrote:

How did you create Inputs/amdgpu-kernel-2.o? We generally want to avoid checking in binary files, so if it is created by llvm-mc or something, we'd like to check in the source file instead of the binary file.

It was created using "llc -filetype=obj...". Unfortunately we do not have an assembler for that architecture so cannot use llvm-mc.

We are also working on more changes to e_flags (related to architectures). Once we agree on those and add support to llvm, I was planning to replace this binary file with the source file (few weeks away). Would that be ok?

[ruiu]

Yes, when you complete your other changes, please replace the file. LGTM

ELF/Arch/AMDGPU.cpp

51

toString(F) is better than F->getName() because an archive file name is included in the former output if F was pulled out from an archive.

ELF/Arch/AMDGPU.cpp

 using namespace llvm::ELF;
 using namespace lld;
 using namespace lld::elf;
 
 namespace {
 class AMDGPU final : public TargetInfo {
 public:
   AMDGPU();
+  uint32_t calcEFlags() const override;
   void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
   RelExpr getRelExpr(RelType Type, const SymbolBody &S,
                      const uint8_t *Loc) const override;
 };
 } // namespace
 
 AMDGPU::AMDGPU() {
   RelativeRel = R_AMDGPU_RELATIVE64;
   GotRel = R_AMDGPU_ABS64;
   GotEntrySize = 8;
 }
 
+uint32_t AMDGPU::calcEFlags() const {
+  if (ObjectFiles.empty())
+    return 0;
+
+  uint32_t Ret =
+    cast<ObjFile<ELF64LE>>(ObjectFiles.front())->getObj().getHeader()->e_flags;
+
+  // Verify that all input files have the same e_flags.
+  for (InputFile *F : ArrayRef<InputFile*>(ObjectFiles).slice(1)) {
+    if (Ret != cast<ObjFile<ELF64LE>>(F)->getObj().getHeader()->e_flags) {
+      error("incompatible e_flags: " + F->getName());
+      return 0;
+    }
+  }
+
+  return Ret;
+}
+
 void AMDGPU::relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const {
   switch (Type) {
   case R_AMDGPU_ABS32:
   case R_AMDGPU_GOTPCREL:
   case R_AMDGPU_GOTPCREL32_LO:
   case R_AMDGPU_REL32:
   case R_AMDGPU_REL32_LO:
     write32le(Loc, Val);

test/ELF/Inputs/amdgpu-kernel-0.s

+.text
+.globl kernel_0
+.align 64
+.amdgpu_hsa_kernel kernel_0
+kernel_0:
+  s_endpgm

test/ELF/Inputs/amdgpu-kernel-1.s

+.text
+.globl kernel_1
+.align 64
+.amdgpu_hsa_kernel kernel_1
+kernel_1:
+  s_endpgm

test/ELF/amdgpu-elf-flags-err.s

+# RUN: llvm-mc -triple amdgcn-amd-amdhsa -mcpu=gfx803 -filetype=obj %S/Inputs/amdgpu-kernel-0.s -o %t-0.o
+# RUN: llvm-mc -triple amdgcn-amd-amdhsa -mcpu=gfx803 -filetype=obj %S/Inputs/amdgpu-kernel-1.s -o %t-1.o
+# RUN: not ld.lld -shared %t-0.o %t-1.o %S/Inputs/amdgpu-kernel-2.o -o %t.so 2>&1 | FileCheck %s
+
+# REQUIRES: amdgpu
+
+# CHECK: error: incompatible e_flags: {{.*}}amdgpu-kernel-2.o

test/ELF/amdgpu-elf-flags.s

+# RUN: llvm-mc -triple amdgcn-amd-amdhsa -mcpu=gfx803 -filetype=obj %S/Inputs/amdgpu-kernel-0.s -o %t-0.o
+# RUN: llvm-mc -triple amdgcn-amd-amdhsa -mcpu=gfx803 -filetype=obj %S/Inputs/amdgpu-kernel-1.s -o %t-1.o
+# RUN: ld.lld -shared %t-0.o %t-1.o -o %t.so
+# RUN: llvm-readobj -file-headers %t.so | FileCheck %s
+
+# REQUIRES: amdgpu
+
+# CHECK: Flags [ (0x2)
+# CHECK:   EF_AMDGPU_ARCH_GCN (0x2)
+# CHECK: ]