[PATCH] D81988: [ELF] Refactor ObjFile<ELFT>::initializeSymbols to enforce the invariant: InputFile::symbols has non null entry

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 19 09:12:43 PDT 2020


MaskRay marked 4 inline comments as done.
MaskRay added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:1101
+  // Symbol resolution of non-local symbols.
+  for (size_t i = firstGlobal, end = eSyms.size(); i != end; ++i) {
+    const Elf_Sym &eSym = eSyms[i];
----------------
jhenderson wrote:
> Isn't this going to result in reading every global symbol twice? That seems like a negative performance hit to me, and I'm not sure it's necessary either (the two loops could still be a single loop it seems to me).
There were two loops before and two loops now. After refactoring there is strictly less work: the second loops starts from firstGlobal (was 0).

> (the two loops could still be a single loop it seems to me)

It will be very difficult.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81988/new/

https://reviews.llvm.org/D81988





More information about the llvm-commits mailing list