[llvm-commits] [llvm] r154497 - in /llvm/trunk: include/llvm/Metadata.h lib/VMCore/LLVMContextImpl.h lib/VMCore/Metadata.cpp

[Benjamin Kramer]

On 11.04.2012, at 22:14, Bill Wendling wrote:

> On Apr 11, 2012, at 7:06 AM, Benjamin Kramer <benny.kra at googlemail.com> wrote:
> 
>> Author: d0k
>> Date: Wed Apr 11 09:06:54 2012
>> New Revision: 154497
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=154497&view=rev
>> Log:
>> Cache the hash value of the operands in the MDNode.
>> 
>> FoldingSet is implemented as a chained hash table. When there is a hash
>> collision during insertion, which is common as we fill the table until a
>> load factor of 2.0 is hit, we walk the chained elements, comparing every
>> operand with the new element's operands. This can be very expensive if the
>> MDNode has many operands.
>> 
>> We sacrifice a word of space in MDNode to cache the full hash value, reducing
>> compares on collision to a minimum. MDNode grows from 28 to 32 bytes + operands
>> on x86. On x86_64 the new bits fit nicely into existing padding, not growing
>> the struct at all.
>> 
>> The actual speedup depends a lot on the test case and is typically between
>> 1% and 2% for C++ code with clang -c -O0 -g.
>> 
>> Modified:
>>   llvm/trunk/include/llvm/Metadata.h
>>   llvm/trunk/lib/VMCore/LLVMContextImpl.h
>>   llvm/trunk/lib/VMCore/Metadata.cpp
>> 
>> Modified: llvm/trunk/include/llvm/Metadata.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Metadata.h?rev=154497&r1=154496&r2=154497&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Metadata.h (original)
>> +++ llvm/trunk/include/llvm/Metadata.h Wed Apr 11 09:06:54 2012
>> @@ -75,6 +75,10 @@
>>  void operator=(const MDNode &);        // DO NOT IMPLEMENT
>>  friend class MDNodeOperand;
>>  friend class LLVMContextImpl;
>> +  friend struct FoldingSetTrait<MDNode>;
>> +
>> +  /// NumOperands - If the MDNode is uniqued cache the hash to speed up lookup.
>        ^^^^^^^^^^^^ - Should be Hash. :)

Oops! Fixed in r154527.

- Ben

>> +  unsigned Hash;
>> 
>>  /// NumOperands - This many 'MDNodeOperand' items are co-allocated onto the
>>  /// end of this MDNode.
>> 
> 
> -bw
> 
>