This is an archive of the discontinued LLVM Phabricator instance.

Move symbol resolution code out of SymbolTable class.
ClosedPublic

Authored by ruiu on May 14 2019, 6:26 AM.

Details

Summary

This is the last patch of the series of patches to make it possible to
resolve symbols without asking SymbolTable to do so.

The main point of this patch is the introduction of
elf::resolveSymbol(Symbol *Old, Symbol *New). That function resolves
or merges given symbols by examining symbol types and call
replaceSymbol (which memcpy's New to Old) if necessary.

With the new function, we have now separated symbol resolution from
symbol lookup. If you already have a Symbol pointer, you can directly
resolve the symbol without asking SymbolTable to do that.

Now that the nice abstraction become available, I can start working on
performance improvement of the linker. As a starter, I'm thinking of
making --{start,end}-lib faster.

--{start,end}-lib is currently unnecessarily slow because it looks up
the symbol table twice for each symbol.

  • The first hash table lookup/insertion occurs when we instantiate a LazyObject file to insert LazyObject symbols.
  • The second hash table lookup/insertion occurs when we create an ObjFile from LazyObject file. That overwrites LazyObject symbols with Defined symbols.

I think it is not too hard to see how we can now eliminate the second
hash table lookup. We can keep LazyObject symbols in Step 1, and then
call elf::resolveSymbol() to do Step 2.

Event Timeline

ruiu created this revision.May 14 2019, 6:26 AM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay added inline comments.May 14 2019, 7:26 AM
lld/ELF/SymbolTable.cpp
580

Have you checked if this chain of dyn_cast is as efficient as a switch dispatch on New->kind()?

getSymbolSize below dispatches on SymbolKind.

lld/ELF/Symbols.h
431

Sym->kind()

ruiu marked 2 inline comments as done.May 15 2019, 9:14 PM
ruiu added inline comments.
lld/ELF/SymbolTable.cpp
580

Done.

lld/ELF/Symbols.h
431

Done.

ruiu updated this revision to Diff 199741.May 15 2019, 9:14 PM
  • review comments
ruiu updated this revision to Diff 199742.May 15 2019, 9:18 PM
  • minor fix
MaskRay accepted this revision.May 15 2019, 9:30 PM
MaskRay added inline comments.
lld/ELF/SymbolTable.cpp
605

symbol kind?

lld/ELF/Symbols.h
449

Does const Symbol &New work? Other add* functions use const references, e.g.

static void addShared(Symbol *Old, const SharedSymbol &New) {

This revision is now accepted and ready to land.May 15 2019, 9:30 PM
ruiu marked 2 inline comments as done.May 16 2019, 6:47 PM
ruiu added inline comments.
lld/ELF/SymbolTable.cpp
605

Done

lld/ELF/Symbols.h
449

Done.

This revision was automatically updated to reflect the committed changes.