[PATCH] D13814: Add support for SHF_MERGE sections

[Rui Ueyama via llvm-commits]

ruiu added a comment.

I don't think splitting the loop would produce substantially duplicated code. Even if code in total gets larger, it's a good thing to do as long as each loop becomes more clear. I didn't try that myself though. You can leave it as is for now if you want. I'll take a look at that later.


================
Comment at: ELF/InputSection.h:83-87
@@ +82,7 @@
+  typedef InputSectionBase<ELFT> Base;
+  typedef typename Base::Elf_Shdr Elf_Shdr;
+  typedef typename llvm::object::ELFFile<ELFT>::Elf_Rela Elf_Rela;
+  typedef typename llvm::object::ELFFile<ELFT>::Elf_Rel Elf_Rel;
+  typedef typename Base::Elf_Sym Elf_Sym;
+  typedef typename Base::uintX_t uintX_t;
+
----------------
Mix of Base and llvm::object::ElfFile<ELFT> looks a bit odd. I'd always use llvm::object::ElfFile<ELFT> because that is one less indirection.

================
Comment at: ELF/OutputSections.cpp:467-468
@@ -448,1 +466,4 @@
+    Offset += getAddend<ELFT>(RI);
+  Offset -= (Offset % Section->getSectionHdr()->sh_entsize);
+  return VA + cast<MergeInputSection<ELFT>>(Section)->getOffset(Offset);
 }
----------------
Is this correct? Offset is a rounded value, and the return value does not seem to include the information you discarded using the % operator.

================
Comment at: ELF/OutputSections.h:208
@@ +207,3 @@
+private:
+  // This map is used to find if we already have an etry for a given value and,
+  // if so, at what offset it is.
----------------
entry (typo)


http://reviews.llvm.org/D13814