[llvm-commits] PATCH: add ContextualDenseMap

Nick Lewycky nicholas at mxc.ca
Mon Aug 30 22:30:24 PDT 2010


Chris Lattner wrote:
>
> On Aug 30, 2010, at 2:52 PM, Nick Lewycky wrote:
>
>> On 30 August 2010 14:03, Chris Lattner <clattner at apple.com
>> <mailto:clattner at apple.com>> wrote:
>>
>>
>>     On Aug 26, 2010, at 8:22 AM, Nick Lewycky wrote:
>>
>>     > This patch adds a new ContextualDenseMap and sinks most of the
>>     dense map logic into DenseMapImpl. A ContextualDenseMap takes a
>>     ContextualDenseMapInfo pointer instead of just a typename. I need
>>     this for mergefuncs where I want a comparison operation that
>>     involves target data.
>>     >
>>     > The four map info functions (getTombstoneKey, getEmptyKey,
>>     getHashValue and isEqual) are made virtual in DenseMapImpl and
>>     then implemented once in DenseMap and again in ContextualDenseMap.
>>     This implementation is modelled after the relationship between
>>     ContextualFoldingSet and FoldingSet.
>>
>>     This seems like it will slow down densemap, perhaps substantially,
>>     by introducing virtual methods.
>>
>>
>> That matches my reaction when looking at FoldingSet but I decided that
>> someone else probably knows more than I do. I'll change this, and
>> probably FoldingSet too.
>
> DenseMap and FoldingSet are used by very different clients. The cost of
> the virtual method in FoldingSet is far less of an issue because the
> cost of populating Profile info is much higher than the cost of
> computing a hash value.
>
>>     It's also not clear to me what this is actually doing. Please
>>     explain what you are trying to do and why the current densemap
>>     can't do it.
>>
>>
>> My implementation of isEqual for MergeFunctions requires a TargetData
>> object. Because the existing DenseMap takes a type and not an object,
>> and the DenseMapInfo methods are all static, I can't give it a
>> TargetData* to use unless I'm willing to stick it in a global
>> variable. (And if you prefer that, let me know; I'd ruled it out.)
>
> Why not make the value in the hash table a struct that contains a
> Function* and a TargetData* (and a cached hash value)?

Thanks Chris, that works great.

Nick



More information about the llvm-commits mailing list