[llvm] r278600 - [ADT] Add a reserve method to DenseSet as well as an insert() for R-value

[Yaron Keren via llvm-commits]

These are bots managed by Takumi outside llvm.org, they don't email
committers.

  http://bb.pgr.jp/grid

I was watching these and the LLVM bots after committing r278602 and noticed
the new failure. Thanks for fixing!


2016-08-13 23:24 GMT+03:00 Mehdi Amini <mehdi.amini at apple.com>:

> Thanks Yaron, reverted in r278603. I didn’t get an email from the bot
> though?
>
> I’ve been bitten by this in the past with std::make_pair and MSVC
> introducing an extra move.
> Will rework this…
>
> —
> Mehdi
>
>
>
>
> On Aug 13, 2016, at 1:07 PM, Yaron Keren <yaron.keren at gmail.com> wrote:
>
> following this commit a bot is failing:
>
> http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux
> http://bb.pgr.jp/builders/cmake-llvm-x86_64-linux/builds/41483
>
>
> 2016-08-13 22:40 GMT+03:00 Mehdi Amini via llvm-commits <
> llvm-commits at lists.llvm.org>:
>
>> Author: mehdi_amini
>> Date: Sat Aug 13 14:40:13 2016
>> New Revision: 278600
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=278600&view=rev
>> Log:
>> [ADT] Add a reserve method to DenseSet as well as an insert() for R-value
>>
>> Modified:
>>     llvm/trunk/include/llvm/ADT/DenseSet.h
>>     llvm/trunk/unittests/ADT/DenseSetTest.cpp
>>
>> Modified: llvm/trunk/include/llvm/ADT/DenseSet.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>> ADT/DenseSet.h?rev=278600&r1=278599&r2=278600&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/ADT/DenseSet.h (original)
>> +++ llvm/trunk/include/llvm/ADT/DenseSet.h Sat Aug 13 14:40:13 2016
>> @@ -58,6 +58,10 @@ public:
>>    /// the Size of the set.
>>    void resize(size_t Size) { TheMap.resize(Size); }
>>
>> +  /// Grow the DenseSet so that it can contain at least \p NumEntries
>> items
>> +  /// before resizing again.
>> +  void reserve(size_t Size) { TheMap.reserve(Size); }
>> +
>>    void clear() {
>>      TheMap.clear();
>>    }
>> @@ -154,6 +158,11 @@ public:
>>      return TheMap.insert(std::make_pair(V, Empty));
>>    }
>>
>> +  std::pair<iterator, bool> insert(ValueT &&V) {
>> +    detail::DenseSetEmpty Empty;
>> +    return TheMap.insert(std::make_pair(std::move(V), Empty));
>> +  }
>> +
>>    /// Alternative version of insert that uses a different (and possibly
>> less
>>    /// expensive) key type.
>>    template <typename LookupKeyT>
>>
>> Modified: llvm/trunk/unittests/ADT/DenseSetTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT
>> /DenseSetTest.cpp?rev=278600&r1=278599&r2=278600&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/ADT/DenseSetTest.cpp (original)
>> +++ llvm/trunk/unittests/ADT/DenseSetTest.cpp Sat Aug 13 14:40:13 2016
>> @@ -65,4 +65,67 @@ TEST(DenseSetCustomTest, FindAsTest) {
>>    EXPECT_TRUE(set.find_as("d") == set.end());
>>  }
>>
>> +// Simple class that counts how many moves and copy happens when growing
>> a map
>> +struct CountCopyAndMove {
>> +  static int Move;
>> +  static int Copy;
>> +  int Value;
>> +  CountCopyAndMove(int Value) : Value(Value) {}
>> +
>> +  CountCopyAndMove(const CountCopyAndMove &) { Copy++; }
>> +  CountCopyAndMove &operator=(const CountCopyAndMove &) {
>> +    Copy++;
>> +    return *this;
>> +  }
>> +  CountCopyAndMove(CountCopyAndMove &&) { Move++; }
>> +  CountCopyAndMove &operator=(const CountCopyAndMove &&) {
>> +    Move++;
>> +    return *this;
>> +  }
>> +};
>> +int CountCopyAndMove::Copy = 0;
>> +int CountCopyAndMove::Move = 0;
>> +} // anonymous namespace
>> +
>> +namespace llvm {
>> +// Specialization required to insert a CountCopyAndMove into a DenseSet.
>> +template <> struct DenseMapInfo<CountCopyAndMove> {
>> +  static inline CountCopyAndMove getEmptyKey() { return
>> CountCopyAndMove(-1); };
>> +  static inline CountCopyAndMove getTombstoneKey() {
>> +    return CountCopyAndMove(-2);
>> +  };
>> +  static unsigned getHashValue(const CountCopyAndMove &Val) {
>> +    return Val.Value;
>> +  }
>> +  static bool isEqual(const CountCopyAndMove &LHS,
>> +                      const CountCopyAndMove &RHS) {
>> +    return LHS.Value == RHS.Value;
>> +  }
>> +};
>> +}
>> +
>> +namespace {
>> +// Make sure reserve actually gives us enough buckets to insert N items
>> +// without increasing allocation size.
>> +TEST(DenseSetCustomTest, ReserveTest) {
>> +  // Test a few different size, 48 is *not* a random choice: we need a
>> value
>> +  // that is 2/3 of a power of two to stress the grow() condition, and
>> the power
>> +  // of two has to be at least 64 because of minimum size allocation in
>> the
>> +  // DenseMa. 66 is a value just above the 64 default init.
>> +  for (auto Size : {1, 2, 48, 66}) {
>> +    DenseSet<CountCopyAndMove> Set;
>> +    Set.reserve(Size);
>> +    unsigned MemorySize = Set.getMemorySize();
>> +    CountCopyAndMove::Copy = 0;
>> +    CountCopyAndMove::Move = 0;
>> +    for (int i = 0; i < Size; ++i)
>> +      Set.insert(CountCopyAndMove(i));
>> +    // Check that we didn't grow
>> +    EXPECT_EQ(MemorySize, Set.getMemorySize());
>> +    // Check that move was called the expected number of times
>> +    EXPECT_EQ(Size, CountCopyAndMove::Move);
>> +    // Check that no copy occured
>> +    EXPECT_EQ(0, CountCopyAndMove::Copy);
>> +  }
>> +}
>>  }
>>
>>
>> _______________________________________________
>> 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/20160813/d18f8fcc/attachment.html>