[PATCH] D17920: Query the StringMap only once when creating MDString (NFC)
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Sun Mar 6 17:38:13 PST 2016
"emplace" is actively misleading, because this doesn't actually do the
same thing that "emplace" does in other pair/map-like structures.
I chose "emplace_second" because the method emplaces the pair::second
element; but if you don't like the name, I suspect David still doesn't
like the semantics; std::piecewise_construct might be the best
approach.
> On 2016-Mar-06, at 17:14, Mehdi Amini <mehdi.amini at apple.com> wrote:
>
> Isn't "_second" weird for a variadic template?
> I mean you can call it:
>
> auto I = Store.emplace_second(Key, arg1, arg2, arg3);
>
> --
> Mehdi
>
>> On Mar 6, 2016, at 5:03 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>
>> +dblaikie
>>
>> If you rename the method `emplace_second` (since you're only emplacing the
>> second element, not the full value), then I'm happy with this. However,
>> David didn't like this patch when I proposed it a year or so ago.
>>
>> IIRC, at the time David wanted a std::pair-style `emplace` in
>> StringMapEntry complete with support for std::piecewise_construct_t. Then
>> you could call:
>> --
>> auto I = Store.emplace(std::piecewise_construct,
>> std::make_tuple(Str),
>> std::make_tuple());
>> --
>>
>> I still think `emplace_second` is sufficiently clear since StringMap is so
>> specialized (and the above looks like boilerplate to me), but this would
>> be an uncontentious path forward.
>>
>>
>>> On 2016-Mar-06, at 14:28, Mehdi AMINI <mehdi.amini at apple.com> wrote:
>>>
>>> joker.eph created this revision.
>>> joker.eph added a reviewer: dexonsmith.
>>> joker.eph added a subscriber: llvm-commits.
>>>
>>> Loading IR with debug info improves MDString::get() from 19ms to 10ms.
>>> This is a rework of D16597 with adding an "emplace" method on the StringMap
>>> to avoid requiring the MDString move ctor to be public.
>>>
>>> http://reviews.llvm.org/D17920
>>>
>>> Files:
>>> include/llvm/ADT/StringMap.h
>>> include/llvm/IR/Metadata.h
>>> lib/IR/Metadata.cpp
>>>
>>> <D17920.49925.patch>
>>
>
More information about the llvm-commits
mailing list