<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thanks Yaron, reverted in r278603. I didn’t get an email from the bot though?<div class=""><br class=""></div><div class="">I’ve been bitten by this in the past with std::make_pair and MSVC introducing an extra move.</div><div class="">Will rework this…</div><div class=""><br class=""></div><div class="">— </div><div class="">Mehdi</div><div class=""><br class=""></div><div class=""><br class=""><div class=""><br class=""></div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 13, 2016, at 1:07 PM, Yaron Keren <<a href="mailto:yaron.keren@gmail.com" class="">yaron.keren@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="rtl" class=""><div dir="ltr" class="">following this commit a bot is failing:</div><div dir="ltr" class=""><br class=""></div><div dir="ltr" class=""><a href="http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux" class="">http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux</a><br class=""></div><div dir="ltr" class=""><a href="http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux/builds/41483" class="">http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux/builds/41483</a><br class=""></div><div dir="ltr" class=""><br class=""></div></div><div class="gmail_extra"><br class=""><div class="gmail_quote"><div dir="ltr" class="">2016-08-13 22:40 GMT+03:00 Mehdi Amini via llvm-commits <span dir="ltr" class=""><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>></span>:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mehdi_amini<br class="">
Date: Sat Aug 13 14:40:13 2016<br class="">
New Revision: 278600<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=278600&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project?rev=278600&view=rev</a><br class="">
Log:<br class="">
[ADT] Add a reserve method to DenseSet as well as an insert() for R-value<br class="">
<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/ADT/<wbr class="">DenseSet.h<br class="">
llvm/trunk/unittests/ADT/<wbr class="">DenseSetTest.cpp<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/ADT/<wbr class="">DenseSet.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseSet.h?rev=278600&r1=278599&r2=278600&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/llvm/trunk/include/<wbr class="">llvm/ADT/DenseSet.h?rev=<wbr class="">278600&r1=278599&r2=278600&<wbr class="">view=diff</a><br class="">
==============================<wbr class="">==============================<wbr class="">==================<br class="">
--- llvm/trunk/include/llvm/ADT/<wbr class="">DenseSet.h (original)<br class="">
+++ llvm/trunk/include/llvm/ADT/<wbr class="">DenseSet.h Sat Aug 13 14:40:13 2016<br class="">
@@ -58,6 +58,10 @@ public:<br class="">
/// the Size of the set.<br class="">
void resize(size_t Size) { TheMap.resize(Size); }<br class="">
<br class="">
+ /// Grow the DenseSet so that it can contain at least \p NumEntries items<br class="">
+ /// before resizing again.<br class="">
+ void reserve(size_t Size) { TheMap.reserve(Size); }<br class="">
+<br class="">
void clear() {<br class="">
TheMap.clear();<br class="">
}<br class="">
@@ -154,6 +158,11 @@ public:<br class="">
return TheMap.insert(std::make_pair(<wbr class="">V, Empty));<br class="">
}<br class="">
<br class="">
+ std::pair<iterator, bool> insert(ValueT &&V) {<br class="">
+ detail::DenseSetEmpty Empty;<br class="">
+ return TheMap.insert(std::make_pair(<wbr class="">std::move(V), Empty));<br class="">
+ }<br class="">
+<br class="">
/// Alternative version of insert that uses a different (and possibly less<br class="">
/// expensive) key type.<br class="">
template <typename LookupKeyT><br class="">
<br class="">
Modified: llvm/trunk/unittests/ADT/<wbr class="">DenseSetTest.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseSetTest.cpp?rev=278600&r1=278599&r2=278600&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-<wbr class="">project/llvm/trunk/unittests/<wbr class="">ADT/DenseSetTest.cpp?rev=<wbr class="">278600&r1=278599&r2=278600&<wbr class="">view=diff</a><br class="">
==============================<wbr class="">==============================<wbr class="">==================<br class="">
--- llvm/trunk/unittests/ADT/<wbr class="">DenseSetTest.cpp (original)<br class="">
+++ llvm/trunk/unittests/ADT/<wbr class="">DenseSetTest.cpp Sat Aug 13 14:40:13 2016<br class="">
@@ -65,4 +65,67 @@ TEST(DenseSetCustomTest, FindAsTest) {<br class="">
EXPECT_TRUE(set.find_as("d") == set.end());<br class="">
}<br class="">
<br class="">
+// Simple class that counts how many moves and copy happens when growing a map<br class="">
+struct CountCopyAndMove {<br class="">
+ static int Move;<br class="">
+ static int Copy;<br class="">
+ int Value;<br class="">
+ CountCopyAndMove(int Value) : Value(Value) {}<br class="">
+<br class="">
+ CountCopyAndMove(const CountCopyAndMove &) { Copy++; }<br class="">
+ CountCopyAndMove &operator=(const CountCopyAndMove &) {<br class="">
+ Copy++;<br class="">
+ return *this;<br class="">
+ }<br class="">
+ CountCopyAndMove(<wbr class="">CountCopyAndMove &&) { Move++; }<br class="">
+ CountCopyAndMove &operator=(const CountCopyAndMove &&) {<br class="">
+ Move++;<br class="">
+ return *this;<br class="">
+ }<br class="">
+};<br class="">
+int CountCopyAndMove::Copy = 0;<br class="">
+int CountCopyAndMove::Move = 0;<br class="">
+} // anonymous namespace<br class="">
+<br class="">
+namespace llvm {<br class="">
+// Specialization required to insert a CountCopyAndMove into a DenseSet.<br class="">
+template <> struct DenseMapInfo<CountCopyAndMove> {<br class="">
+ static inline CountCopyAndMove getEmptyKey() { return CountCopyAndMove(-1); };<br class="">
+ static inline CountCopyAndMove getTombstoneKey() {<br class="">
+ return CountCopyAndMove(-2);<br class="">
+ };<br class="">
+ static unsigned getHashValue(const CountCopyAndMove &Val) {<br class="">
+ return Val.Value;<br class="">
+ }<br class="">
+ static bool isEqual(const CountCopyAndMove &LHS,<br class="">
+ const CountCopyAndMove &RHS) {<br class="">
+ return LHS.Value == RHS.Value;<br class="">
+ }<br class="">
+};<br class="">
+}<br class="">
+<br class="">
+namespace {<br class="">
+// Make sure reserve actually gives us enough buckets to insert N items<br class="">
+// without increasing allocation size.<br class="">
+TEST(DenseSetCustomTest, ReserveTest) {<br class="">
+ // Test a few different size, 48 is *not* a random choice: we need a value<br class="">
+ // that is 2/3 of a power of two to stress the grow() condition, and the power<br class="">
+ // of two has to be at least 64 because of minimum size allocation in the<br class="">
+ // DenseMa. 66 is a value just above the 64 default init.<br class="">
+ for (auto Size : {1, 2, 48, 66}) {<br class="">
+ DenseSet<CountCopyAndMove> Set;<br class="">
+ Set.reserve(Size);<br class="">
+ unsigned MemorySize = Set.getMemorySize();<br class="">
+ CountCopyAndMove::Copy = 0;<br class="">
+ CountCopyAndMove::Move = 0;<br class="">
+ for (int i = 0; i < Size; ++i)<br class="">
+ Set.insert(CountCopyAndMove(i)<wbr class="">);<br class="">
+ // Check that we didn't grow<br class="">
+ EXPECT_EQ(MemorySize, Set.getMemorySize());<br class="">
+ // Check that move was called the expected number of times<br class="">
+ EXPECT_EQ(Size, CountCopyAndMove::Move);<br class="">
+ // Check that no copy occured<br class="">
+ EXPECT_EQ(0, CountCopyAndMove::Copy);<br class="">
+ }<br class="">
+}<br class="">
}<br class="">
<br class="">
<br class="">
______________________________<wbr class="">_________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/<wbr class="">mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></div></body></html>