ELF/AMDGPU: Give symbols in the .hsatext section section relative offsets

[ruiu]

Hi Tom,

You are adding the feature to the old LLD ELF linker. Is this intentional? We are focusing on the new one right now, and it is pretty likely that the new one will take over the old one soon.

lib/ReaderWriter/ELF/AMDGPU/AMDGPUELFFile.h

+//===- lib/ReaderWriter/ELF/AMDGPUELFFile.h -------------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#ifndef LLD_READER_WRITER_ELF_AMDGPU_AMDGPU_ELF_FILE_H
+#define LLD_READER_WRITER_ELF_AMDGPU_AMDGPU_ELF_FILE_H
+
+#include "ELFReader.h"
+
+namespace lld {
+namespace elf {
+
+class AMDGPUELFFile;
+
+class AMDGPUELFDefinedAtom : public ELFDefinedAtom<ELF64LE> {
+  typedef llvm::object::Elf_Sym_Impl<ELF64LE> Elf_Sym;
+  typedef llvm::object::Elf_Shdr_Impl<ELF64LE> Elf_Shdr;
+
+public:
+  bool HasSectionRelativeOffset;
+  int64_t SectionRelOffset;
+
+  AMDGPUELFDefinedAtom(const AMDGPUELFFile &file, StringRef symbolName,
+                       StringRef sectionName, const Elf_Sym *symbol,
+                       const Elf_Shdr *section, ArrayRef<uint8_t> contentData,
+                       unsigned int referenceStart, unsigned int referenceEnd,
+                       std::vector<ELFReference<ELF64LE> *> &referenceList,
+                       bool HasSectionRelativeOffset = false);
+};
+
+class AMDGPUELFFile : public ELFFile<ELF64LE> {
+public:
+  AMDGPUELFFile(std::unique_ptr<MemoryBuffer> mb, ELFLinkingContext &ctx);
+
+private:
+  typedef llvm::object::Elf_Sym_Impl<ELF64LE> Elf_Sym;
+  typedef llvm::object::Elf_Shdr_Impl<ELF64LE> Elf_Shdr;
+  typedef typename llvm::object::ELFFile<ELF64LE>::Elf_Rel Elf_Rel;
+  typedef typename llvm::object::ELFFile<ELF64LE>::Elf_Rela Elf_Rela;
+
+  ELFDefinedAtom<ELF64LE> *createDefinedAtom(
+      StringRef symName, StringRef sectionName, const Elf_Sym *sym,
+      const Elf_Shdr *sectionHdr, ArrayRef<uint8_t> contentData,
+      unsigned int referenceStart, unsigned int referenceEnd,
+      std::vector<ELFReference<ELF64LE> *> &referenceList) override;
+};
+}
+}
+
+#endif

lib/ReaderWriter/ELF/AMDGPU/AMDGPUELFFile.cpp

+//===- lib/ReaderWriter/ELF/AMDGPUELFFile.cpp -----------------------------===//
+//
+//                             The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "AMDGPUELFFile.h"
+#include "llvm/Support/Debug.h"
+
+namespace lld {
+namespace elf {
+
+AMDGPUELFDefinedAtom::AMDGPUELFDefinedAtom(
+    const AMDGPUELFFile &file, StringRef symbolName, StringRef sectionName,
+    const Elf_Sym *symbol, const Elf_Shdr *section,
+    ArrayRef<uint8_t> contentData, unsigned int referenceStart,
+    unsigned int referenceEnd,
+    std::vector<ELFReference<ELF64LE> *> &referenceList,
+    bool hasSectionRelativeOffset)
+    : ELFDefinedAtom<ELF64LE>(file, symbolName, sectionName, symbol, section,
+                              contentData, referenceStart, referenceEnd,
+                              referenceList),
+      HasSectionRelativeOffset(hasSectionRelativeOffset) {
+  if (HasSectionRelativeOffset)
+    SectionRelOffset = symbol->st_value;
+}
+
+AMDGPUELFFile::AMDGPUELFFile(std::unique_ptr<MemoryBuffer> mb,
+                             ELFLinkingContext &ctx)
+    : ELFFile<ELF64LE>(std::move(mb), ctx) {}
+
+ELFDefinedAtom<ELF64LE> *AMDGPUELFFile::createDefinedAtom(
+    StringRef symName, StringRef sectionName, const Elf_Sym *sym,
+    const Elf_Shdr *sectionHdr, ArrayRef<uint8_t> contentData,
+    unsigned int referenceStart, unsigned int referenceEnd,
+    std::vector<ELFReference<ELF64LE> *> &referenceList) {
+
+  // The st_value field in the symbol table for symbols in the .hsatext
+  // section must be section relative, so we need to store the original
+  // symbol offset, which is section relative, so we can write it to the
+  // symbol table in AMDDGPUSymbolTable::finalize().
+  bool HasSectionRelativeOffset = false;
+  if (sectionName == ".hsatext")
+    HasSectionRelativeOffset = true;
+
+  return new (this->_readerStorage) AMDGPUELFDefinedAtom(
+      *this, symName, sectionName, sym, sectionHdr, contentData, referenceStart,
+      referenceEnd, referenceList, HasSectionRelativeOffset);
+}
+
+} // elf
+} // lld

lib/ReaderWriter/ELF/AMDGPU/AMDGPUSymbolTable.h

 class AMDGPUSymbolTable : public SymbolTable<ELF64LE> {
 public:
   typedef llvm::object::Elf_Sym_Impl<ELF64LE> Elf_Sym;
 
   AMDGPUSymbolTable(const ELFLinkingContext &ctx);
 
   void addDefinedAtom(Elf_Sym &sym, const DefinedAtom *da,
                       int64_t addr) override;
+  void finalize(bool sort) override;
 };
 
 } // elf
 } // lld
 
 #endif

lib/ReaderWriter/ELF/AMDGPU/AMDGPUSymbolTable.cpp

 //===--------- lib/ReaderWriter/ELF/AMDGPU/AMDGPUSymbolTable.cpp ----------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "AMDGPUSymbolTable.h"
-#include "ELFFile.h"
+#include "AMDGPUELFFile.h"
 #include "Atoms.h"
 #include "SectionChunks.h"
 
 using namespace lld;
 using namespace lld::elf;
 
 AMDGPUSymbolTable::AMDGPUSymbolTable(const ELFLinkingContext &ctx)
     : SymbolTable(ctx, ".symtab", TargetLayout<ELF64LE>::ORDER_SYMBOL_TABLE) {}
 
 void AMDGPUSymbolTable::addDefinedAtom(Elf_Sym &sym, const DefinedAtom *da,
                                        int64_t addr) {
   SymbolTable::addDefinedAtom(sym, da, addr);
 
-  // Make st_value section relative.
-  // FIXME: This is hack to give kernel symbols a section relative offset.
-  // Because of this hack only on kernel can be included in a binary file.
-  sym.st_value = 0;
+}
+
+void AMDGPUSymbolTable::finalize(bool sort) {
+  SymbolTable::finalize(sort);
+
+   for (SymbolEntry &STE : _symbolTable) {
+     if (!STE._atom) {
+       continue;
+     }
+    const AMDGPUELFDefinedAtom *atom = dyn_cast<const AMDGPUELFDefinedAtom>(STE._atom);
+    if (!atom)
+      continue;
+    if (atom->HasSectionRelativeOffset)
+      STE._symbol.st_value = atom->SectionRelOffset;
+   }
 }

lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.h

 //===- lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.h ------------------===//
 //
 //                             The LLVM Linker
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef AMDGPU_TARGET_HANDLER_H
 #define AMDGPU_TARGET_HANDLER_H
 
-#include "ELFFile.h"
+#include "AMDGPUELFFile.h"
 #include "ELFReader.h"
 #include "AMDGPURelocationHandler.h"
 #include "TargetLayout.h"
 
 namespace lld {
 namespace elf {
 class AMDGPULinkingContext;
 

 public:
   AMDGPUTargetHandler(AMDGPULinkingContext &targetInfo);
 
   const TargetRelocationHandler &getRelocationHandler() const override {
     return *_relocationHandler;
   }
 
   std::unique_ptr<Reader> getObjReader() override {
-    return llvm::make_unique<ELFReader<ELFFile<ELF64LE>>>(_ctx);
+    return llvm::make_unique<ELFReader<AMDGPUELFFile>>(_ctx);
   }
 
   std::unique_ptr<Reader> getDSOReader() override {
     return llvm::make_unique<ELFReader<DynamicFile<ELF64LE>>>(_ctx);
   }
 
   std::unique_ptr<Writer> getWriter() override;
 

lib/ReaderWriter/ELF/AMDGPU/CMakeLists.txt

 add_llvm_library(lldAMDGPUELFTarget
+  AMDGPUELFFile.cpp
   AMDGPUExecutableWriter.cpp
   AMDGPULinkingContext.cpp
   AMDGPURelocationHandler.cpp
   AMDGPUSymbolTable.cpp
   AMDGPUTargetHandler.cpp
   LINK_LIBS
     lldELF
     lldReaderWriter
     lldCore
     LLVMObject
     LLVMSupport
   )

test/elf/AMDGPU/hsa-multiple-kernels.test

+# RUN: yaml2obj -format=elf %s > %t.obj
+# RUN: lld -flavor gnu -target amdgcn--hsa %t.obj -o %t.exe --noinhibit-exec
+# RUN: llvm-readobj -h -program-headers -s -symbols %t.exe | FileCheck %s
+
+# CHECK: ElfHeader {
+# CHECK:  Ident {
+# CHECK: Class: 64-bit (0x2)
+# CHECK: DataEncoding: LittleEndian (0x1)
+# CHECK: Machine: EM_AMDGPU (0xE0)
+
+# CHECK: Symbol {
+# CHECK: Name: kernel0
+# CHECK: Value: 0x0
+# CHECK: Binding: Global (0x1)
+# CHECK: Type: AMDGPU_HSA_KERNEL (0xA)
+# CHECK: }
+# CHECK: Symbol {
+# CHECK: Name: kernel1
+# CHECK: Value: 0x200
+# CHECK: Binding: Global (0x1)
+# CHECK: Type: AMDGPU_HSA_KERNEL (0xA)
+# CHECK: }
+
+---
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  OSABI:           ELFOSABI_GNU
+  Type:            ET_REL
+  Machine:         EM_AMDGPU
+Sections:
+  - Name:            .hsatext
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_AMDGPU_HSA_AGENT, SHF_AMDGPU_HSA_CODE,
+                       SHF_EXECINSTR, SHF_WRITE ]
+    AddressAlign:    256
+    Content:         ''
+    Size:            1024
+Symbols:
+  Global:
+    - Name:            kernel0
+      Type:            STT_GNU_IFUNC
+      Section:         .hsatext
+      Value:           0
+      Size:            0
+    - Name:            kernel1
+      Type:            STT_GNU_IFUNC
+      Section:         .hsatext
+      Value:           512
+      Size:            0
+...