[PATCH] D53393: Add a addAbsolute static function to Writer.cpp

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

First, let me explain my motivation as to why I made changes to the symbol table in the first place, so that you understand the background of my changes.

One of the most important goals of lld is speed, and we know that hash table lookup is one of the slowest operations in the linker because hash table lookup is not very fast and we have massive number of symbols to be inserted to a hash table. In lld, we strictly do only one symbol table lookup per a symbol, which I believe the minimum number of hash table lookup, to make the linker as fast as possible.

But I noticed a few weeks ago that we do insert the same symbol more than once if we are using --start-lib and --end-lib options. These options give archive file semantics to object files between the options, so that you can avoid using "ar" command (which tend to be slow) to create archive files from object files. Let's call object files between --{start,end}-lib lazy objects. In lld, when we visit lazy objects for the first time, we add lazy symbols for lazy objects as placeholders, to memorize that symbols in lazy objects can be resolvable if we really need them. When the linker knows that it needs lazy objects, it then create real symbols (defined and undefined) to replace lazy symbols. To do that, it looks up the symbol table again with the same set of symbols. That is a violation of our design policy.

So, I started fixing it, and noticed that that's not very easy to do, because all SymbolTable functions takes not a Symbol but a symbol name (which is StringRef) as a key. As long as we are using that interface, we cannot avoid hash table lookups. I also noticed that SymbolTable has too many utility functions that are not really necessary, which makes it hard to make changes to the code. So, I started off with simplifying the class first.

That is the motivation to reduce number of SymbolTable's member functions. My goal was not achieved yet, but I needed to simplify it first.

Back to your change, this change itself seems neutral to me from the readability perspective, but it is *very* likely to "break" again, possibly soon, because this is not really a public API or anything, and I didn't finish my job there yet. Are you fine with that? You can always write the same function in your local patch.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53393