[llvm-commits] [llvm] r158639 - in /llvm/trunk: include/llvm/ADT/DenseMap.h unittests/ADT/DenseMapTest.cpp
Chandler Carruth
chandlerc at gmail.com
Sun Jun 17 03:33:51 PDT 2012
Author: chandlerc
Date: Sun Jun 17 05:33:51 2012
New Revision: 158639
URL: http://llvm.org/viewvc/llvm-project?rev=158639&view=rev
Log:
Add tests for *DenesMap for both key and value types' construction and
destruction and fix a bug in SmallDenseMap they caught.
This is kind of a poor-man's version of the testing that just adds the
addresses to a set on construction and removes them on destruction. We
check that double construction and double destruction don't occur.
Amusingly enough, this is enough to catch a lot of SmallDenseMap issues
because we spend a lot of time with fixed stable addresses in the inline
buffer.
The SmallDenseMap bug fix included makes grow() not double-destroy in
some cases. It also fixes a FIXME there, the code was pretty crappy. We
now don't have any wasted initialization, but we do move the entries in
inline bucket array an extra time. It's probably a better tradeoff, and
is much easier to get correct.
Modified:
llvm/trunk/include/llvm/ADT/DenseMap.h
llvm/trunk/unittests/ADT/DenseMapTest.cpp
Modified: llvm/trunk/include/llvm/ADT/DenseMap.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/DenseMap.h?rev=158639&r1=158638&r2=158639&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/DenseMap.h (original)
+++ llvm/trunk/include/llvm/ADT/DenseMap.h Sun Jun 17 05:33:51 2012
@@ -310,7 +310,6 @@
std::swap(getNumTombstones(), RHS.getNumTombstones());
}
-private:
static unsigned getHashValue(const KeyT &Val) {
return KeyInfoT::getHashValue(Val);
}
@@ -325,6 +324,7 @@
return KeyInfoT::getTombstoneKey();
}
+private:
unsigned getNumEntries() const {
return static_cast<const DerivedT *>(this)->getNumEntries();
}
@@ -803,24 +803,34 @@
if (AtLeast <= InlineBuckets)
return; // Nothing to do.
- // First grow an allocated bucket array in another map and move our
- // entries into it.
- // FIXME: This is wasteful, we don't need the inline buffer here, and we
- // certainly don't need to initialize it to empty.
- SmallDenseMap TmpMap;
- TmpMap.Small = false;
- new (TmpMap.getLargeRep()) LargeRep(allocateBuckets(AtLeast));
- TmpMap.moveFromOldBuckets(getInlineBuckets(),
- getInlineBuckets()+InlineBuckets);
-
- // Now steal the innards back into this map, and arrange for the
- // temporary map to be cleanly torn down.
- assert(NumEntries == TmpMap.NumEntries);
+ // First move the inline buckets into a temporary storage.
+ typename AlignedCharArray<BucketT[InlineBuckets]>::union_type
+ TmpStorage;
+ BucketT *TmpBegin = reinterpret_cast<BucketT *>(TmpStorage.buffer);
+ BucketT *TmpEnd = TmpBegin;
+
+ // Loop over the buckets, moving non-empty, non-tombstones into the
+ // temporary storage. Have the loop move the TmpEnd forward as it goes.
+ const KeyT EmptyKey = this->getEmptyKey();
+ const KeyT TombstoneKey = this->getTombstoneKey();
+ for (BucketT *P = getBuckets(), *E = P + InlineBuckets; P != E; ++P) {
+ if (!KeyInfoT::isEqual(P->first, EmptyKey) &&
+ !KeyInfoT::isEqual(P->first, TombstoneKey)) {
+ assert((TmpEnd - TmpBegin) < InlineBuckets &&
+ "Too many inline buckets!");
+ new (&TmpEnd->first) KeyT(llvm_move(P->first));
+ new (&TmpEnd->second) ValueT(llvm_move(P->second));
+ ++TmpEnd;
+ P->second.~ValueT();
+ }
+ P->first.~KeyT();
+ }
+
+ // Now make this map use the large rep, and move all the entries back
+ // into it.
Small = false;
- NumTombstones = llvm_move(TmpMap.NumTombstones);
- new (getLargeRep()) LargeRep(llvm_move(*TmpMap.getLargeRep()));
- TmpMap.getLargeRep()->~LargeRep();
- TmpMap.Small = true;
+ new (getLargeRep()) LargeRep(allocateBuckets(AtLeast));
+ this->moveFromOldBuckets(TmpBegin, TmpEnd);
return;
}
Modified: llvm/trunk/unittests/ADT/DenseMapTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/DenseMapTest.cpp?rev=158639&r1=158638&r2=158639&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/DenseMapTest.cpp (original)
+++ llvm/trunk/unittests/ADT/DenseMapTest.cpp Sun Jun 17 05:33:51 2012
@@ -10,6 +10,7 @@
#include "gtest/gtest.h"
#include "llvm/ADT/DenseMap.h"
#include <map>
+#include <set>
using namespace llvm;
@@ -29,6 +30,47 @@
return &dummy_arr1[i];
}
+/// \brief A test class that tries to check that construction and destruction
+/// occur correctly.
+class CtorTester {
+ static std::set<CtorTester *> Constructed;
+ int Value;
+
+public:
+ explicit CtorTester(int Value = 0) : Value(Value) {
+ EXPECT_TRUE(Constructed.insert(this).second);
+ }
+ CtorTester(uint32_t Value) : Value(Value) {
+ EXPECT_TRUE(Constructed.insert(this).second);
+ }
+ CtorTester(const CtorTester &Arg) : Value(Arg.Value) {
+ EXPECT_TRUE(Constructed.insert(this).second);
+ }
+ ~CtorTester() {
+ EXPECT_EQ(1u, Constructed.erase(this));
+ }
+ operator uint32_t() const { return Value; }
+
+ int getValue() const { return Value; }
+ bool operator==(const CtorTester &RHS) const { return Value == RHS.Value; }
+};
+
+std::set<CtorTester *> CtorTester::Constructed;
+
+struct CtorTesterMapInfo {
+ static inline CtorTester getEmptyKey() { return CtorTester(-1); }
+ static inline CtorTester getTombstoneKey() { return CtorTester(-2); }
+ static unsigned getHashValue(const CtorTester &Val) {
+ return Val.getValue() * 37u;
+ }
+ static bool isEqual(const CtorTester &LHS, const CtorTester &RHS) {
+ return LHS == RHS;
+ }
+};
+
+CtorTester getTestKey(int i, CtorTester *) { return CtorTester(i); }
+CtorTester getTestValue(int i, CtorTester *) { return CtorTester(42 + i); }
+
// Test fixture, with helper functions implemented by forwarding to global
// function overloads selected by component types of the type parameter. This
// allows all of the map implementations to be tested with shared
@@ -57,8 +99,11 @@
// Register these types for testing.
typedef ::testing::Types<DenseMap<uint32_t, uint32_t>,
DenseMap<uint32_t *, uint32_t *>,
+ DenseMap<CtorTester, CtorTester, CtorTesterMapInfo>,
SmallDenseMap<uint32_t, uint32_t>,
- SmallDenseMap<uint32_t *, uint32_t *>
+ SmallDenseMap<uint32_t *, uint32_t *>,
+ SmallDenseMap<CtorTester, CtorTester, 4,
+ CtorTesterMapInfo>
> DenseMapTestTypes;
TYPED_TEST_CASE(DenseMapTest, DenseMapTestTypes);
More information about the llvm-commits
mailing list