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

[Reid Kleckner via llvm-commits]

I'll go ahead and revert for now to fix the bot. Its unclear to me what we
should do long term.

On Sat, Apr 21, 2018 at 6:11 PM Chandler Carruth <chandlerc at gmail.com>
wrote:

> 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/20180424/4bd975a6/attachment.html>