clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed.
================ Comment at: lldb/source/Utility/ConstString.cpp:177-183 uint8_t hash(const llvm::StringRef &s) const { - uint32_t h = llvm::djbHash(s); + return hash(StringPool::hash(s)); + } + + uint8_t hash(uint32_t h) const { return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff; } ---------------- Can we rename these functions to something other than "hash"? It is a bit confusing to have a function called "hash" that returns a uint8_t when what this is really doing is getting use the string pool index. Maybe "GetPoolIndex"? I could see this function accidentally being used to hash a string instead of using StringPool::hash() ================ Comment at: llvm/include/llvm/ADT/StringMap.h:69 + /// Overload that explicitly takes precomputed hash(Key). + unsigned LookupBucketFor(StringRef Key, unsigned FullHashValue); ---------------- Use "uint32_t" instead of unsigned to clarify the exact width of the integer. Or create a "HashType" typedef and then use that. ================ Comment at: llvm/include/llvm/ADT/StringMap.h:106 + static unsigned hash(StringRef Key) { return llvm::djbHash(Key, 0); } + ---------------- ================ Comment at: llvm/lib/Support/StringMap.cpp:83 +unsigned StringMapImpl::LookupBucketFor(StringRef Name, + unsigned FullHashValue) { +#ifdef EXPENSIVE_CHECKS ---------------- ================ Comment at: llvm/lib/Support/StringMap.cpp:141 /// This does not modify the map. -int StringMapImpl::FindKey(StringRef Key) const { +int StringMapImpl::FindKey(StringRef Key, unsigned FullHashValue) const { if (NumBuckets == 0) ---------------- CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122974/new/ https://reviews.llvm.org/D122974 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits