<div class="gmail_quote">On Tue, Mar 6, 2012 at 3:02 AM, Jay Foad <span dir="ltr"><<a href="mailto:jay.foad@gmail.com">jay.foad@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">>> --- llvm/trunk/lib/VMCore/ConstantsContext.h (original)<br>
>> +++ llvm/trunk/lib/VMCore/ConstantsContext.h Tue Mar  6 04:43:52 2012<br>
>> @@ -16,6 +16,7 @@<br>
>>   #define LLVM_CONSTANTSCONTEXT_H<br>
>><br>
>>   #include "llvm/ADT/DenseMap.h"<br>
>> +#include "llvm/ADT/Hashing.h"<br>
>>   #include "llvm/InlineAsm.h"<br>
>>   #include "llvm/Instructions.h"<br>
>>   #include "llvm/Operator.h"<br>
>> @@ -656,48 +657,20 @@<br>
>>         return ConstantClassInfo::getTombstoneKey();<br>
>>       }<br>
>>       static unsigned getHashValue(const ConstantClass *CP) {<br>
>> -      // This is adapted from SuperFastHash by Paul Hsieh.<br>
>> -      unsigned Hash = TypeClassInfo::getHashValue(CP->getType());<br>
>> -      for (unsigned I = 0, E = CP->getNumOperands(); I<  E; ++I) {<br>
>> -        unsigned Data = ConstantInfo::getHashValue(CP->getOperand(I));<br>
>> -        Hash         += Data&  0xFFFF;<br>
>> -        unsigned Tmp  = ((Data>>  16)<<  11) ^ Hash;<br>
>> -        Hash          = (Hash<<  16) ^ Tmp;<br>
>> -        Hash         += Hash>>  11;<br>
>> -      }<br>
>> -<br>
>> -      // Force "avalanching" of final 127 bits.<br>
>> -      Hash ^= Hash<<  3;<br>
>> -      Hash += Hash>>  5;<br>
>> -      Hash ^= Hash<<  4;<br>
>> -      Hash += Hash>>  17;<br>
>> -      Hash ^= Hash<<  25;<br>
>> -      Hash += Hash>>  6;<br>
>> -      return Hash;<br>
>> +      hash_code code = hash_value(CP->getType());<br>
>> +      for (unsigned I = 0, E = CP->getNumOperands(); I<  E; ++I)<br>
>> +        code = hash_combine(code, hash_value(CP->getOperand(I)));<br>
><br>
> maybe this could/should be done with hash_combine_range?<br>
<br>
</div></div>As Meador explained when he posted the patch :-) the operands array is<br>
an array of Uses, but we don't want to hash the whole of each Use,<br>
just the Value* pointer.<br></blockquote><div><br></div><div>I think you underestimate how slow it is to hash things using incremental calls to hash_combine. The reason for the other interface is that it is *much* faster.</div>
<div><br></div><div>Also, in the second case, we already had an array of pointers that we could chew through with hash_combine_range. We can make both interfaces look the same with a SmallVector buffer. I've switched the code in r152200.</div>
<div><br></div><div>It most cases it is cheaper to build a buffer, and hash it once, than to call hash_combine incrementally.</div><div><br></div><div>FYI, feel free to ping me on any hashing reviews if there are questions about how best to use the interface.</div>
</div>