[PATCH] D11267: LLD: Add AMDGPU ELF ReaderWriter

[Rui Ueyama]

ruiu added inline comments.

================
Comment at: lib/ReaderWriter/ELF/AMDGPU/AMDGPUELFFile.cpp:14-22
@@ +13,10 @@
+
+bool AMDGPUELFFile::handleSectionWithNoSymbols(
+    const Elf_Shdr *shdr, std::vector<const Elf_Sym *> &syms) const {
+  // We want to create atoms for note sections, because these sections
+  // contain information required by the hsa runtime.
+  if (shdr && shdr->sh_type == llvm::ELF::SHT_NOTE && syms.empty())
+    return true;
+
+  return ELFFile::handleSectionWithNoSymbols(shdr, syms);
+}
----------------
Can you add llvm::ELF::SHT_NOTE to ELFFile::handleSectionWithNoSymbols and remove this function? I'm not 100% sure but I believe doing that is not harmful to other ports.

================
Comment at: lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.cpp:55
@@ +54,3 @@
+  TargetLayout::assignSectionsToSegments();
+  for (auto osi : _outputSections) {
+    for (auto section : osi->sections()) {
----------------
Use real type instead of `auto` as the type is not obvious.

================
Comment at: lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.cpp:56
@@ +55,3 @@
+  for (auto osi : _outputSections) {
+    for (auto section : osi->sections()) {
+      StringRef InputSectionName = section->inputSectionName();
----------------
Ditto

================
Comment at: lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.h:39
@@ +38,3 @@
+                TargetLayout::SectionOrder sectionOrder) override {
+
+    if (name == ".note") {
----------------
Remove this empty line.

================
Comment at: lib/ReaderWriter/ELF/AMDGPU/AMDGPUTargetHandler.h:43-45
@@ +42,5 @@
+    }
+    if (name == ".text") {
+      return new (_allocator) HSATextSection(_ctx);
+    }
+    return TargetLayout::createSection(name, contentType, contentPermissions,
----------------
You may want to move these lines above `if (name == ".note")` because this statement does not use `contentType` and returns immediately.


http://reviews.llvm.org/D11267