[llvm] 2c84b92 - Fix assertion in SmallDenseMap constructor with reserve from non-power-of-2 buckets count
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 25 10:09:52 PDT 2022
Author: Vladislav Dzhidzhoev
Date: 2022-07-25T17:09:44Z
New Revision: 2c84b92346bc45a88f92d781e30daa8490049c4d
URL: https://github.com/llvm/llvm-project/commit/2c84b92346bc45a88f92d781e30daa8490049c4d
DIFF: https://github.com/llvm/llvm-project/commit/2c84b92346bc45a88f92d781e30daa8490049c4d.diff
LOG: Fix assertion in SmallDenseMap constructor with reserve from non-power-of-2 buckets count
`SmallDenseMap` constructor with reserve gets an arbitrary `NumInitBuckets` value and passes it below to `init` method.
If `NumInitBuckets` is greater then `InlineBuckets`, then `SmallDenseMap` initializes to large representation passing `NumInitBuckets` below to `DenseMap` initialization. `DenseMap::initEmpty` method asserts that initial buckets count must be a power of 2.
Proposed solution is to update `NumInitBuckets` value in `SmallDenseMap` constructor till the next power of 2. It should satisfy both `DenseMap` preconditions and required minimum buckets count for reservation.
Reviewed By: atrick
Differential Revision: https://reviews.llvm.org/D129825
Added:
Modified:
llvm/include/llvm/ADT/DenseMap.h
llvm/unittests/ADT/DenseMapTest.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index c14414c46419c..a6d19d342822f 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -907,6 +907,8 @@ class SmallDenseMap
public:
explicit SmallDenseMap(unsigned NumInitBuckets = 0) {
+ if (NumInitBuckets > InlineBuckets)
+ NumInitBuckets = NextPowerOf2(NumInitBuckets - 1);
init(NumInitBuckets);
}
diff --git a/llvm/unittests/ADT/DenseMapTest.cpp b/llvm/unittests/ADT/DenseMapTest.cpp
index 4dd314c5c9019..619ddb05f269f 100644
--- a/llvm/unittests/ADT/DenseMapTest.cpp
+++ b/llvm/unittests/ADT/DenseMapTest.cpp
@@ -607,6 +607,15 @@ TEST(DenseMapCustomTest, LargeSmallDenseMapCompaction) {
EXPECT_TRUE(map.find(0) == map.end());
}
+TEST(DenseMapCustomTest, SmallDenseMapWithNumBucketsNonPowerOf2) {
+ // Is not power of 2.
+ const unsigned NumInitBuckets = 33;
+ // Power of 2 less then NumInitBuckets.
+ constexpr unsigned InlineBuckets = 4;
+ // Constructor should not trigger assert.
+ SmallDenseMap<int, int, InlineBuckets> map(NumInitBuckets);
+}
+
TEST(DenseMapCustomTest, TryEmplaceTest) {
DenseMap<int, std::unique_ptr<int>> Map;
std::unique_ptr<int> P(new int(2));
More information about the llvm-commits
mailing list