[llvm-commits] PATCH: add ContextualDenseMap

[Nick Lewycky]

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.


> 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.)

I use the TargetData to determine whether or not two GEPs are going to
advance the pointer by the same number of bytes. There is some lousy
fallback code for when TargetData is not available, and I could try
enhancing that instead, but it will never work as well as just using the TD.

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