[llvm] r300477 - [APInt] Remove self move check from move assignment operator

[Chandler Carruth via llvm-commits]

Sorry to do some commit necromancy...

But our Windows expensive check bots fail because of this:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/9193/steps/test-check-all/logs/stdio

It seems that while std::stable_sort no longer does self moves,
std::shuffle still does.

Thoughts?
-Chandler

On Mon, Apr 17, 2017 at 11:57 AM Craig Topper via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: ctopper
> Date: Mon Apr 17 13:44:27 2017
> New Revision: 300477
>
> URL: http://llvm.org/viewvc/llvm-project?rev=300477&view=rev
> Log:
> [APInt] Remove self move check from move assignment operator
>
> 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.
>
> Differential Revision: https://reviews.llvm.org/D32116
>
>
>
> Modified:
>     llvm/trunk/include/llvm/ADT/APInt.h
>     llvm/trunk/unittests/ADT/APIntTest.cpp
>
> Modified: llvm/trunk/include/llvm/ADT/APInt.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/APInt.h?rev=300477&r1=300476&r2=300477&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/APInt.h (original)
> +++ llvm/trunk/include/llvm/ADT/APInt.h Mon Apr 17 13:44:27 2017
> @@ -693,24 +693,16 @@ public:
>
>    /// @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;
>    }
>
> Modified: llvm/trunk/unittests/ADT/APIntTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/APIntTest.cpp?rev=300477&r1=300476&r2=300477&view=diff
>
> ==============================================================================
> --- llvm/trunk/unittests/ADT/APIntTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/APIntTest.cpp Mon Apr 17 13:44:27 2017
> @@ -1606,36 +1606,6 @@ TEST(APIntTest, 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());
> @@ -2025,3 +1995,5 @@ TEST(APIntTest, GCD) {
>    APInt C = GreatestCommonDivisor(A, B);
>    EXPECT_EQ(C, HugePrime);
>  }
> +
> +} // end anonymous namespace
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180422/9794c6bc/attachment.html>