[llvm-commits] [llvm] r162222 - /llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

[Chandler Carruth]

On Tue, Aug 21, 2012 at 9:54 AM, Sean Silva <silvas at purdue.edu> wrote:

> > DenseSet is implemented as a DenseMap<foo, char>, if you stick a pointer
> in
> > there every entry is sizeof(void *) + 1 + padding = 2 * sizeof(void*), a
> > significant memory overhead.
>
> Currently DenseMap uses just a regular pair<K,V> as the bucket type.
> If this were replaced with something like libc++'s __compressed_pair,
> then you could just use an empty struct as the mapped_type and this
> overhead would go away. Does LLVM have something like
> __compressed_pair already? I can't seem to find one, but I'd like a
> second opinion.
>
> If it doesn't, then I'll put together a patch adding a compressed
> pair, and then fixup DenseMap to use it. It seems like almost every
> use of DenseSet in the LLVM/Clang codebase would benefit from this
> (bucket array would become half as big!).
>

I'm not at all sure this complexity is worth it. Are you sure that the
DenseSet instances in the codebase are really suffering? I think we use
SmallPtrSet (which doesn't have this problem IIRC) much more commonly.

If you have some compelling reason to invest so much energy here, I would
rather see a direct set implementation, with a map implementation layered
on top of it, than compressed pair hacking to make a map actually look more
like a set.


>
> --Sean Silva
>
> On Tue, Aug 21, 2012 at 8:23 AM, Benjamin Kramer <benny.kra at gmail.com>
> wrote:
> >
> > On 21.08.2012, at 05:19, Sean Silva <silvas at purdue.edu> wrote:
> >
> >> Would SmallDenseMap be more efficient here since SmallPtrSet does a
> >> linear scan? There's also a SmallPtrSet of size 64 in the DAG combiner
> >> that I think would significantly benefit from becoming a SmallDenseMap
> >> (64 means that when it is pretty large but still in its small storage
> >> it does a linear scan over up to 8 cache lines).
> >>
> >> More generally, in what situations is a SmallPtrSet going to be
> >> preferable to a SmallDenseMap?
> >
> > For only 8 entries the entire linearly scanned buffer fits into 8*8=64
> bytes on
> > x86_64, which should be really fast even with a linear scan, maybe even
> faster
> > than doing a hash lookup.
> >
> > For 64*8 bytes the situation is different, that SmallPtrSet should be
> either
> > shrunk or replaced with a SmallDenseMap (which carries its own overhead
> because
> > it stores keys and values).
> >
> > DenseSet is implemented as a DenseMap<foo, char>, if you stick a pointer
> in
> > there every entry is sizeof(void *) + 1 + padding = 2 * sizeof(void*), a
> > significant memory overhead.
> >
> > Of course, if malloc thrashing in turn dwarves this overhead,
> SmallDenseMap
> > would be a good idea. Real numbers would be nice but I guess it's hard to
> > find a test case where llvm spends a lot of time in badly configured
> > SmallPtrSets.
> >
> > - Ben
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120821/3a844ef9/attachment.html>