[llvm] r215478 - APInt: Make self-move-assignment a no-op to fix stage3 clang-cl

[David Blaikie]

On Tue, Aug 12, 2014 at 3:01 PM, Reid Kleckner <reid at kleckner.net> wrote:
> Author: rnk
> Date: Tue Aug 12 17:01:39 2014
> New Revision: 215478
>
> URL: http://llvm.org/viewvc/llvm-project?rev=215478&view=rev
> Log:
> APInt: Make self-move-assignment a no-op to fix stage3 clang-cl
>
> It's not clear what the semantics of a self-move should be.  The
> consensus appears to be that a self-move should leave the object in a
> moved-from state, which is what our existing move assignment operator
> does.
>
> However, the MSVC 2013 STL will perform self-moves in some cases.  In
> particular, when doing a std::stable_sort of an already sorted APSInt
> vector of an appropriate size, one of the merge steps will self-move
> half of the elements.
>
> We don't notice this when building with MSVC, because MSVC will not
> synthesize the move assignment operator for APSInt.  Presumably

I'm assuming you're going to look into this a bit further, though?
(other than just 'presumably') both to understand when/how long we
need this workaround in place, and whether we need to do anything to
Clang for MSVC compatibility here?

> MSVC
> does this because APInt, the base class, has user-declared special
> members that implicitly delete move special members.  Instead, MSVC
> selects the copy-assign operator, which defends against self-assignment.
> Clang, on the other hand, selects the move-assign operator, and we get
> garbage APInts.

I'm curious how Clang synthesizes a move assignment operator if the
base class's move assignment is implicitly deleted. Any idea what's
going on there?

>
> 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=215478&r1=215477&r2=215478&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ADT/APInt.h (original)
> +++ llvm/trunk/include/llvm/ADT/APInt.h Tue Aug 12 17:01:39 2014
> @@ -656,13 +656,22 @@ public:
>
>    /// @brief Move assignment operator.
>    APInt &operator=(APInt &&that) {
> -    if (!isSingleWord())
> +    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;
>        delete[] pVal;
> +    }
>
> -    BitWidth = that.BitWidth;
>      VAL = that.VAL;
>
> +    // If 'this == &that', avoid zeroing our own bitwidth by storing to 'that'
> +    // first.
> +    unsigned ThatBitWidth = 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=215478&r1=215477&r2=215478&view=diff
> ==============================================================================
> --- llvm/trunk/unittests/ADT/APIntTest.cpp (original)
> +++ llvm/trunk/unittests/ADT/APIntTest.cpp Tue Aug 12 17:01:39 2014
> @@ -678,4 +678,21 @@ TEST(APIntTest, nearestLogBase2) {
>    EXPECT_EQ(A9.nearestLogBase2(), UINT32_MAX);
>  }
>
> +TEST(APIntTest, SelfMoveAssignment) {
> +  APInt X(32, 0xdeadbeef);
> +  X = std::move(X);
> +  EXPECT_EQ(32, X.getBitWidth());
> +  EXPECT_EQ(0xdeadbeefULL, X.getLimitedValue());
> +
> +  uint64_t Bits[] = {0xdeadbeefdeadbeefULL, 0xdeadbeefdeadbeefULL};
> +  APInt Y(128, Bits);
> +  Y = std::move(Y);
> +  EXPECT_EQ(128, Y.getBitWidth());
> +  EXPECT_EQ(~0ULL, Y.getLimitedValue());
> +  const uint64_t *Raw = Y.getRawData();
> +  EXPECT_EQ(2, Y.getNumWords());
> +  EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[0]);
> +  EXPECT_EQ(0xdeadbeefdeadbeefULL, Raw[1]);
> +}
> +
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits