[PATCH] D31364: LTO: Reduce memory consumption by creating an in-memory symbol table for InputFiles. NFCI.

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added a subscriber: inglorion.
pcc added inline comments.


================
Comment at: lld/COFF/InputFiles.cpp:347
     Symbol *Sym;
-    if (Flags & object::BasicSymbolRef::SF_Undefined) {
+    if (ObjSym.isUndefined()) {
       Sym = Symtab->addUndefined(SymName, this, false);
----------------
rafael wrote:
> These small utility predicates are awesome. Would you mind committing them first with the current implementation? It would be a nice improvement and make this patch easier to read.
Will do


================
Comment at: llvm/include/llvm/Object/IRSymtab.h:31
+
+typedef support::ulittle32_t Word;
+
----------------
rafael wrote:
> why do you use this for Symbol? If we are storing something by value it is better to just use a uint32_t.
If we eventually want to write these data structures out to files I guess we will need to specify an endianness. The plan I have in mind is:
1. define data structures that can be potentially written to files, but use them only as an in-memory format to start with (that is what this patch does)
2. potentially improve them in-tree (e.g. by sharing the string table between the module and the symbol table)
3. flip the switch and start writing them to files


================
Comment at: llvm/lib/LTO/LTO.cpp:601
+      if (!IRName.empty()) {
+        auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
+            IRName, GlobalValue::ExternalLinkage, ""));
----------------
pcc wrote:
> This is not actually NFC but a fix for a bug found by a colleague. I'll split it out into a separate change later.
Correction: this is NFC. The bug is elsewhere. I'll let @inglorion send out a fix in due course.


https://reviews.llvm.org/D31364