[PATCH] D63344: lld/elf: Deduplicate undefined symbol diagnostics

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

I made it so that it prints at most 10 references. Most of the time when I get this diagnostic, it's because the symbol doesn't exist, and seeing many references to it isn't useful.



================
Comment at: lld/ELF/Relocations.cpp:720
+};
+static std::vector<UndefinedDiag>& Undefs() {
+  // Function-local static to not require a global initializer.
----------------
ruiu wrote:
> nit: add a blank line
> 
> In lld we honestly don't care too much about eliminating global initializers, so it can probably be just a global variable, but you need to erase the contents before the first use so that multiple runs of lld::elf::main in the same process works as expected.
Made it a global. (You might want to reconsider it at some point, though. The main cost of static initializers, other than that initialization order is undefined, is that the code for the initializer needs to be paged in. For example, the coff linker needs to load all the bits of the elf linker that is for static initializers, even though they'll never be used. See also https://web.archive.org/web/20160811161720/https://blog.mozilla.org/tglek/2010/05/27/startup-backward-constructors/ / https://glandium.org/blog/?cat=13&paged=3)


================
Comment at: lld/ELF/Relocations.cpp:818
+
+  {
+    static std::mutex Mu;
----------------
ruiu wrote:
> Is this really called by a multithreaded code?
Hm, I hadn't really checked, I had believed the comment on the bug. It's called from here http://llvm-cs.pcc.me.uk/tools/lld/ELF/Writer.cpp#1741 , but it looks like forEachRelSec() doesn't do any parallel for loops, so probably not. Removed the lock.


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

https://reviews.llvm.org/D63344