<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Jan 26, 2016 at 1:40 PM, Mehdi Amini <span dir="ltr"><<a href="mailto:mehdi.amini@apple.com" target="_blank">mehdi.amini@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Jan 26, 2016, at 1:30 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Tue, Jan 26, 2016 at 12:34 PM, Mehdi AMINI via llvm-commits<span> </span><span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">joker.eph created this revision.<br>joker.eph added a reviewer: dexonsmith.<br>joker.eph added a subscriber: llvm-commits.<br><br>Loading IR with debug info improves MDString::get() from 19ms to 10ms.<br><br><a href="http://reviews.llvm.org/D16597" rel="noreferrer" target="_blank">http://reviews.llvm.org/D16597</a><br><br>Files:<br> <span> </span>include/llvm/IR/Metadata.h<br> <span> </span>lib/IR/Metadata.cpp<br><br>Index: lib/IR/Metadata.cpp<br>===================================================================<br>--- lib/IR/Metadata.cpp<br>+++ lib/IR/Metadata.cpp<br>@@ -399,17 +399,12 @@<br><br> MDString *MDString::get(LLVMContext &Context, StringRef Str) {<br>   auto &Store = Context.pImpl->MDStringCache;<br>-  auto I = Store.find(Str);<br>-  if (I != Store.end())<br>-    return &I->second;<br>-<br>-  auto *Entry =<br>-      StringMapEntry<MDString>::Create(Str, Store.getAllocator(), MDString());<br>-  bool WasInserted = Store.insert(Entry);<br>-  (void)WasInserted;<br>-  assert(WasInserted && "Expected entry to be inserted");<br>-  Entry->second.Entry = Entry;<br>-  return &Entry->second;<br>+  auto I = Store.insert(std::make_pair(Str, MDString()));<br>+  auto &MapEntry = I.first->getValue();<br>+  if (!I.second)<br>+    return &MapEntry;<br>+  MapEntry.Entry = &*I.first;<br>+  return &MapEntry;<br> }<br><br> StringRef MDString::getString() const {<br>Index: include/llvm/IR/Metadata.h<br>===================================================================<br>--- include/llvm/IR/Metadata.h<br>+++ include/llvm/IR/Metadata.h<br>@@ -599,9 +599,9 @@<br><br>   StringMapEntry<MDString> *Entry;<br>   MDString() : Metadata(MDStringKind, Uniqued), Entry(nullptr) {}<br>-  MDString(MDString &&) : Metadata(MDStringKind, Uniqued) {}<br></blockquote><div><br></div><div>Well that was weird/scary/bogus ^</div></div></div></blockquote><div><br></div></div></div><div>Yes but private :)</div><span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br> public:<br>+  MDString(MDString &&R) : Metadata(MDStringKind, Uniqued), Entry(R.Entry) {}<br></blockquote><div><br></div><div>Could we just remove this, and rely on the implicit move/copy ctors? ^ I guess maybe the base copy/move ctor doesn't do the right thing (since it's not being used here)<br></div></div></div></blockquote><div><br></div></span><div>Good point!</div><span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div><br>Should we just make this the copy ctor? It doesn't seme to really be moving anything.</div></div></div></blockquote><div><br></div></span><div>We could consider MDString to be a “lightweight” value-based type (like StringRef) and declare a copy constructor, but will it be coherent with the rest of the other metadata types?</div></div></div></blockquote><div><br></div><div>Not sure about the rest of them, but it seems reasonable for this one (if we're going to amke it public at all - at that point we admit that MDString is much like StringRef, as you say)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>I was not really happy to make it public in the first place, but std::pair needs it to insert in the map.</div><div><br></div><div>— </div><span class="HOEnZb"><font color="#888888"><div>Mehdi</div></font></span><span class=""><div><br></div><div><br></div><div><br></div><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">   static MDString *get(LLVMContext &Context, StringRef Str);<br>   static MDString *get(LLVMContext &Context, const char *Str) {<br>     return get(Context, Str ? StringRef(Str) : StringRef());<br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></span></div><br></div></blockquote></div><br></div></div>