[llvm-commits] PATCH: add ContextualDenseMap

[Chris Lattner]

On Aug 30, 2010, at 2:52 PM, Nick Lewycky wrote:

> On 30 August 2010 14:03, Chris Lattner <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)?

-Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100830/dd369d15/attachment.html>