[PATCH] D18795: Don't store names in the symbols

[Rui Ueyama via llvm-commits]

ruiu added a comment.

If this is neutral in terms of performance, I wouldn't probably do this, as it feels pretty natural to me to have symbols have names, even though we do not use them often in the linker. It is also useful for debugging and easy to understand.

In addition to that, this change seems to make it hard to parallelize file parsing. Currently, most part of file parsing does not interact with other parts of the linker, so we can call them in parallel. However, if you pass Resolve to each parsing function, it is no longer possible.


================
Comment at: ELF/InputFiles.h:83-84
@@ -82,1 +82,4 @@
 
+  enum WhichSymbols { LOCAL_SYMBOLS, GLOBAL_SYMBOLS, ALL_SYMBOLS };
+  Elf_Sym_Range getElfSymbols(WhichSymbols);
+
----------------
I'd define three functions instead of dispatching three ways in a single function.

================
Comment at: ELF/InputFiles.h:115
@@ -113,2 +114,3 @@
   explicit ObjectFile(MemoryBufferRef M);
-  void parse(llvm::DenseSet<StringRef> &ComdatGroups);
+  void parse(llvm::function_ref<void(StringRef, SymbolBody *)> Resolve,
+             llvm::DenseSet<StringRef> &ComdatGroups);
----------------
Is there any reason to use llvm::function_read instead of std::function?


http://reviews.llvm.org/D18795