[PATCH] D17920: Query the StringMap only once when creating MDString (NFC)

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Sun Mar 6 17:14:28 PST 2016


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