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

[David Blaikie via llvm-commits]

dblaikie added a subscriber: dblaikie.
dblaikie added a comment.

Separate unit test for the StringMap changes might be nice.

Could you remind me what the performance problem was that this is saving? Copying the empty MDString into the StringMapEntry? (what's in an MDString that makes it expensive to copy?) Oh it's the extra map lookup, I see.

std::unordered_map::emplace returns the classic <iterator, bool> pair. Perhaps emplace_second should do the same? Rather than relying on being able to detect the invalid default constructed state of the key?


================
Comment at: include/llvm/IR/Metadata.h:595
@@ -594,3 +594,3 @@
   MDString() : Metadata(MDStringKind, Uniqued), Entry(nullptr) {}
-  MDString(MDString &&) : Metadata(MDStringKind, Uniqued) {}
+  MDString(MDString &&R) = default;
 
----------------
Why do we need a move ctor at all if we're supporting emplace? Can we just delete it? (even if we omit it, it should be removed implicitly - same with the move assignment above, I think)


http://reviews.llvm.org/D17920