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