[PATCH] Make StringMap aware of POD types

[Benjamin Kramer]

On 17.03.2014, at 22:18, Pete Cooper <peter_cooper at apple.com> wrote:

> 
> On Mar 17, 2014, at 1:15 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> 
>> 
>> On 17.03.2014, at 20:38, Chandler Carruth <chandlerc at google.com> wrote:
>> 
>>> 
>>> On Mon, Mar 17, 2014 at 12:32 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>>> I was worried about duplication, but actually that might be cleaner.  I’ll give that a try now.
>>> 
>>> If the duplication is a problem, factor that into a private helper function used by both?
>> 
>> It can be written in two lines, not worth its own function.
> Agreed.
>> 
>> for (auto &Entry : this)
>>  Entry.Destroy(Allocator);
> Thanks for this.  Its a nice small snippet.  I tweaked it to this:
> 
>     if (!empty()) {
>       for (auto &Entry : *this)
>         Entry.Destroy(Allocator);
> 
> However, this is still about 10% faster
> 
> 
>     if (!empty()) {
>       for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
>         StringMapEntryBase *&Bucket = TheTable[I];
>         if (Bucket && Bucket != getTombstoneVal()) {
>           static_cast<MapEntryTy*>(Bucket)->Destroy(Allocator);
>         }
>       }
>     }
> 
> The problem is that iterator++ isn’t getting optimized out.  Actually neither is the whole loop I gave, but somehow it runs a bit quicker.  I think its failing to inline ++ as it shows up in the trace even on -O3.

The latter loop gets eliminated by clang -O3 if I try it in the dtor of a simple StringMap<void*>. It's probably fine to use this more verbose form, eliminating the iterator loop is surprisingly hard, if possible at all.

- Ben