[APInt] Remove self move check from move assignment operator

[craig.topper]

Summary

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.

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.

This reduces the size of the opt binary on my local x86-64 build by about 4k.

[craig.topper]

Fix diff context

[hans]

include/llvm/ADT/APInt.h

695–696

Should we assert that this != &that ?

704

Actually, couldn't we just memcpy/memove both VAL and BitWidth, i.e. the whole APInt? Don't know if that makes any difference though.

705–706

This isn't strictly necessary right, just for finding bugs? Maybe we should only do it for asserts builds?

[craig.topper]

include/llvm/ADT/APInt.h

695–696

I'll add that.

705–706

Setting the bitwidth of 'that' to 0 is needed so 'that' doesn't try to delete the pVal array in its destructor.

[hans]

Oh right, forgot the other one has a destructor :-)

lgtm

include/llvm/ADT/APInt.h

if (isSingleWord() && RHS.isSingleWord()) {
       BitWidth = RHS.BitWidth;
       return clearUnusedBits();
     }
 
     return AssignSlowCase(RHS);
   }
 
   /// @brief Move assignment operator.
   APInt &operator=(APInt &&that) {
-    if (!isSingleWord()) {
-      // The MSVC STL shipped in 2013 requires that self move assignment be a
-      // no-op.  Otherwise algorithms like stable_sort will produce answers
-      // where half of the output is left in a moved-from state.
-      if (this == &that)
-        return *this;
+    assert(this != &that && "Self-move not supported");
+    if (!isSingleWord())
       delete[] pVal;
-    }
 
     // Use memcpy so that type based alias analysis sees both VAL and pVal
     // as modified.
     memcpy(&VAL, &that.VAL, sizeof(uint64_t));
 
-    // If 'this == &that', avoid zeroing our own bitwidth by storing to 'that'
-    // first.
-    unsigned ThatBitWidth = that.BitWidth;
+    BitWidth = that.BitWidth;
     that.BitWidth = 0;
-    BitWidth = ThatBitWidth;
 
     return *this;
   }
 
   /// \brief Assignment operator.
   ///
   /// The RHS value is assigned to *this. If the significant bits in RHS exceed
   /// the bit width, the excess bits are truncated. If the bit width is larger
   /// than 64, the value is zero filled in the unspecified high order bits.

unittests/ADT/APIntTest.cpp

for (int N : { 1, 2, 3, 4, 7, 8, 16, 32, 64, 127, 128, 129, 256 }) {
     }
     for (int I = 1; I < N; ++I) {
       APInt MaskVal = APInt::getHighBitsSet(N, I);
       EXPECT_TRUE(MaskVal.isShiftedMask());
     }
   }
 }
 
-#if defined(__clang__)
-// Disable the pragma warning from versions of Clang without -Wself-move
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wunknown-pragmas"
-// Disable the warning that triggers on exactly what is being tested.
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wself-move"
-#endif
-TEST(APIntTest, SelfMoveAssignment) {
-  APInt X(32, 0xdeadbeef);
-  X = std::move(X);
-  EXPECT_EQ(32u, X.getBitWidth());
-  EXPECT_EQ(0xdeadbeefULL, X.getLimitedValue());
-
-  uint64_t Bits[] = {0xdeadbeefdeadbeefULL, 0xdeadbeefdeadbeefULL};
-  APInt Y(128, Bits);
-  Y = std::move(Y);
-  EXPECT_EQ(128u, Y.getBitWidth());
-  EXPECT_EQ(~0ULL, Y.getLimitedValue());
-  const uint64_t *Raw = Y.getRawData();
-  EXPECT_EQ(2u, Y.getNumWords());
-  EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[0]);
-  EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[1]);
-}
-#if defined(__clang__)
-#pragma clang diagnostic pop
-#pragma clang diagnostic pop
-#endif
-}
-
 TEST(APIntTest, reverseBits) {
   EXPECT_EQ(1, APInt(1, 1).reverseBits());
   EXPECT_EQ(0, APInt(1, 0).reverseBits());
 
   EXPECT_EQ(3, APInt(2, 3).reverseBits());
   EXPECT_EQ(3, APInt(2, 3).reverseBits());
 
   EXPECT_EQ(0xb, APInt(4, 0xd).reverseBits());
TEST(APIntTest, GCD) {
   APInt HugePrime = APInt::getLowBitsSet(BitWidth, 4423);
 
   // 9931 and 123456 are coprime.
   APInt A = HugePrime * APInt(BitWidth, 9931);
   APInt B = HugePrime * APInt(BitWidth, 123456);
   APInt C = GreatestCommonDivisor(A, B);
   EXPECT_EQ(C, HugePrime);
 }
+
+} // end anonymous namespace