[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:03:22 PST 2016


+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