[PATCH] D19025: Hash symbol names only once per global SymbolBody

[Rui Ueyama via llvm-commits]

ruiu added a comment.

This is a very good change -- simple and effective.


================
Comment at: ELF/SymbolTable.h:25
@@ +24,3 @@
+  SymName(StringRef Name) : Name(Name) {
+    HashValue = llvm::DenseMapInfo<StringRef>::getHashValue(Name);
+  }
----------------
Do you have to use DenseMapInfo<StringRef>::getHashValue? I'd just use llvm::hash_value.

================
Comment at: ELF/SymbolTable.h:30
@@ +29,3 @@
+  StringRef Name;
+  unsigned HashValue;
+};
----------------
The size of unsigned int depends on the hosting platform, not the target platform, so SymName would have different hashes on 32-bit and 64-bit. Does this code produces the identical binaries when cross-linking on different machines?

================
Comment at: ELF/SymbolTable.h:30
@@ +29,3 @@
+  StringRef Name;
+  unsigned HashValue;
+};
----------------
ruiu wrote:
> The size of unsigned int depends on the hosting platform, not the target platform, so SymName would have different hashes on 32-bit and 64-bit. Does this code produces the identical binaries when cross-linking on different machines?
I'd name Hash instead of HashValue.


http://reviews.llvm.org/D19025