diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h --- a/llvm/include/llvm/ADT/DenseMap.h +++ b/llvm/include/llvm/ADT/DenseMap.h @@ -1006,13 +1006,10 @@ } void grow(unsigned AtLeast) { - if (AtLeast >= InlineBuckets) + if (AtLeast > InlineBuckets) AtLeast = std::max(64, NextPowerOf2(AtLeast-1)); if (Small) { - if (AtLeast < InlineBuckets) - return; // Nothing to do. - // First move the inline buckets into a temporary storage. AlignedCharArrayUnion TmpStorage; BucketT *TmpBegin = reinterpret_cast(TmpStorage.buffer); @@ -1035,10 +1032,13 @@ P->getFirst().~KeyT(); } - // Now make this map use the large rep, and move all the entries back - // into it. - Small = false; - new (getLargeRep()) LargeRep(allocateBuckets(AtLeast)); + // AtLeast == InlineBuckets can happen if there are many tombstones, + // and grow() is used to remove them. Usually we always switch to the + // large rep here. + if (AtLeast > InlineBuckets) { + Small = false; + new (getLargeRep()) LargeRep(allocateBuckets(AtLeast)); + } this->moveFromOldBuckets(TmpBegin, TmpEnd); return; } diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp --- a/llvm/unittests/ADT/DenseMapTest.cpp +++ b/llvm/unittests/ADT/DenseMapTest.cpp @@ -580,6 +580,24 @@ EXPECT_TRUE(map.find(32) == map.end()); } +TEST(DenseMapCustomTest, LargeSmallDenseMapCompaction) { + SmallDenseMap map; + // Fill to < 3/4 load. + for (unsigned i = 0; i < 95; ++i) + map[i] = i; + // And erase, leaving behind tombstones. + for (unsigned i = 0; i < 95; ++i) + map.erase(i); + // Fill further, so that less than 1/8 are empty, but still below 3/4 load. + for (unsigned i = 95; i < 128; ++i) + map[i] = i; + + EXPECT_EQ(33u, map.size()); + // Similar to the previous test, check for a non-existing element, as an + // indirect check that tombstones have been removed. + EXPECT_TRUE(map.find(0) == map.end()); +} + TEST(DenseMapCustomTest, TryEmplaceTest) { DenseMap> Map; std::unique_ptr P(new int(2));