<div dir="ltr">I'll go ahead and revert for now to fix the bot. Its unclear to me what we should do long term.</div><br><div class="gmail_quote"><div dir="ltr">On Sat, Apr 21, 2018 at 6:11 PM Chandler Carruth <<a href="mailto:chandlerc@gmail.com">chandlerc@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Sorry to do some commit necromancy...<div><br></div><div>But our Windows expensive check bots fail because of this:</div><div><a href="http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/9193/steps/test-check-all/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/9193/steps/test-check-all/logs/stdio</a><br></div><div><br></div><div>It seems that while std::stable_sort no longer does self moves, std::shuffle still does.</div><div><br></div><div>Thoughts?</div><div>-Chandler</div><br><div class="gmail_quote"><div dir="ltr">On Mon, Apr 17, 2017 at 11:57 AM Craig Topper via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: ctopper<br>
Date: Mon Apr 17 13:44:27 2017<br>
New Revision: 300477<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=300477&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=300477&view=rev</a><br>
Log:<br>
[APInt] Remove self move check from move assignment operator<br>
<br>
This was added to work around a bug in MSVC 2013's implementation of stable_sort. That bug has been fixed as of MSVC 2015 so we shouldn't need this anymore.<br>
<br>
Technically the current implementation has undefined behavior because we only protect the deleting of the pVal array with the self move check. There is still a memcpy of that.VAL to VAL that isn't protected. In the case of self move those are the same local and memcpy is undefined for src and dst overlapping.<br>
<br>
This reduces the size of the opt binary on my local x86-64 build by about 4k.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D32116" rel="noreferrer" target="_blank">https://reviews.llvm.org/D32116</a><br>
<br>
<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/APInt.h<br>
llvm/trunk/unittests/ADT/APIntTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/APInt.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=300477&r1=300476&r2=300477&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=300477&r1=300476&r2=300477&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/APInt.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/APInt.h Mon Apr 17 13:44:27 2017<br>
@@ -693,24 +693,16 @@ public:<br>
<br>
/// @brief Move assignment operator.<br>
APInt &operator=(APInt &&that) {<br>
- if (!isSingleWord()) {<br>
- // The MSVC STL shipped in 2013 requires that self move assignment be a<br>
- // no-op. Otherwise algorithms like stable_sort will produce answers<br>
- // where half of the output is left in a moved-from state.<br>
- if (this == &that)<br>
- return *this;<br>
+ assert(this != &that && "Self-move not supported");<br>
+ if (!isSingleWord())<br>
delete[] pVal;<br>
- }<br>
<br>
// Use memcpy so that type based alias analysis sees both VAL and pVal<br>
// as modified.<br>
memcpy(&VAL, &that.VAL, sizeof(uint64_t));<br>
<br>
- // If 'this == &that', avoid zeroing our own bitwidth by storing to 'that'<br>
- // first.<br>
- unsigned ThatBitWidth = that.BitWidth;<br>
+ BitWidth = that.BitWidth;<br>
that.BitWidth = 0;<br>
- BitWidth = ThatBitWidth;<br>
<br>
return *this;<br>
}<br>
<br>
Modified: llvm/trunk/unittests/ADT/APIntTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=300477&r1=300476&r2=300477&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=300477&r1=300476&r2=300477&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ADT/APIntTest.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/APIntTest.cpp Mon Apr 17 13:44:27 2017<br>
@@ -1606,36 +1606,6 @@ TEST(APIntTest, isShiftedMask) {<br>
}<br>
}<br>
<br>
-#if defined(__clang__)<br>
-// Disable the pragma warning from versions of Clang without -Wself-move<br>
-#pragma clang diagnostic push<br>
-#pragma clang diagnostic ignored "-Wunknown-pragmas"<br>
-// Disable the warning that triggers on exactly what is being tested.<br>
-#pragma clang diagnostic push<br>
-#pragma clang diagnostic ignored "-Wself-move"<br>
-#endif<br>
-TEST(APIntTest, SelfMoveAssignment) {<br>
- APInt X(32, 0xdeadbeef);<br>
- X = std::move(X);<br>
- EXPECT_EQ(32u, X.getBitWidth());<br>
- EXPECT_EQ(0xdeadbeefULL, X.getLimitedValue());<br>
-<br>
- uint64_t Bits[] = {0xdeadbeefdeadbeefULL, 0xdeadbeefdeadbeefULL};<br>
- APInt Y(128, Bits);<br>
- Y = std::move(Y);<br>
- EXPECT_EQ(128u, Y.getBitWidth());<br>
- EXPECT_EQ(~0ULL, Y.getLimitedValue());<br>
- const uint64_t *Raw = Y.getRawData();<br>
- EXPECT_EQ(2u, Y.getNumWords());<br>
- EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[0]);<br>
- EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[1]);<br>
-}<br>
-#if defined(__clang__)<br>
-#pragma clang diagnostic pop<br>
-#pragma clang diagnostic pop<br>
-#endif<br>
-}<br>
-<br>
TEST(APIntTest, reverseBits) {<br>
EXPECT_EQ(1, APInt(1, 1).reverseBits());<br>
EXPECT_EQ(0, APInt(1, 0).reverseBits());<br>
@@ -2025,3 +1995,5 @@ TEST(APIntTest, GCD) {<br>
APInt C = GreatestCommonDivisor(A, B);<br>
EXPECT_EQ(C, HugePrime);<br>
}<br>
+<br>
+} // end anonymous namespace<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><br>
</blockquote></div></div>
</blockquote></div>