Skip to content

Commit 53f51da

Browse files
committed
[ADT] Allow K to be incomplete during DenseMap<K*, V> instantiation
DenseMap requires two sentinel values for keys: empty and tombstone values. To avoid undefined behavior, LLVM aligns the two sentinel pointers to alignof(T). This requires T to be complete, which is needlessly restrictive. Instead, assume that DenseMap pointer keys have a maximum alignment of 4096, and use the same sentinel values for all pointer keys. The new sentinels are: empty: static_cast<uintptr_t>(-1) << 12 tombstone: static_cast<uintptr_t>(-2) << 12 These correspond to the addresses of -4096 and -8192. Hopefully, such a key is never inserted into a DenseMap. I encountered this while looking at making clang's SourceManager not require FileManager.h, but it has several maps keyed on classes defined in FileManager.h. FileManager depends on various LLVM FS headers, which cumulatively take ~200ms to parse, and are generally not needed. Reviewed By: hans Differential Revision: https://reviews.llvm.org/D75301
1 parent d369334 commit 53f51da

File tree

2 files changed

+37
-4
lines changed

2 files changed

+37
-4
lines changed

llvm/include/llvm/ADT/DenseMapInfo.h

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
#include "llvm/ADT/ArrayRef.h"
1717
#include "llvm/ADT/Hashing.h"
1818
#include "llvm/ADT/StringRef.h"
19-
#include "llvm/Support/PointerLikeTypeTraits.h"
2019
#include <cassert>
2120
#include <cstddef>
2221
#include <cstdint>
@@ -32,18 +31,28 @@ struct DenseMapInfo {
3231
//static bool isEqual(const T &LHS, const T &RHS);
3332
};
3433

35-
// Provide DenseMapInfo for all pointers.
34+
// Provide DenseMapInfo for all pointers. Come up with sentinel pointer values
35+
// that are aligned to alignof(T) bytes, but try to avoid requiring T to be
36+
// complete. This allows clients to instantiate DenseMap<T*, ...> with forward
37+
// declared key types. Assume that no pointer key type requires more than 4096
38+
// bytes of alignment.
3639
template<typename T>
3740
struct DenseMapInfo<T*> {
41+
// The following should hold, but it would require T to be complete:
42+
// static_assert(alignof(T) <= (1 << Log2MaxAlign),
43+
// "DenseMap does not support pointer keys requiring more than "
44+
// "Log2MaxAlign bits of alignment");
45+
static constexpr uintptr_t Log2MaxAlign = 12;
46+
3847
static inline T* getEmptyKey() {
3948
uintptr_t Val = static_cast<uintptr_t>(-1);
40-
Val <<= PointerLikeTypeTraits<T*>::NumLowBitsAvailable;
49+
Val <<= Log2MaxAlign;
4150
return reinterpret_cast<T*>(Val);
4251
}
4352

4453
static inline T* getTombstoneKey() {
4554
uintptr_t Val = static_cast<uintptr_t>(-2);
46-
Val <<= PointerLikeTypeTraits<T*>::NumLowBitsAvailable;
55+
Val <<= Log2MaxAlign;
4756
return reinterpret_cast<T*>(Val);
4857
}
4958

llvm/unittests/ADT/DenseMapTest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -622,4 +622,28 @@ TEST(DenseMapCustomTest, ConstTest) {
622622
EXPECT_NE(Map.find(B), Map.end());
623623
EXPECT_NE(Map.find(C), Map.end());
624624
}
625+
626+
struct IncompleteStruct;
627+
628+
TEST(DenseMapCustomTest, OpaquePointerKey) {
629+
// Test that we can use a pointer to an incomplete type as a DenseMap key.
630+
// This is an important build time optimization, since many classes have
631+
// DenseMap members.
632+
DenseMap<IncompleteStruct *, int> Map;
633+
int Keys[3] = {0, 0, 0};
634+
IncompleteStruct *K1 = reinterpret_cast<IncompleteStruct *>(&Keys[0]);
635+
IncompleteStruct *K2 = reinterpret_cast<IncompleteStruct *>(&Keys[1]);
636+
IncompleteStruct *K3 = reinterpret_cast<IncompleteStruct *>(&Keys[2]);
637+
Map.insert({K1, 1});
638+
Map.insert({K2, 2});
639+
Map.insert({K3, 3});
640+
EXPECT_EQ(Map.count(K1), 1u);
641+
EXPECT_EQ(Map[K1], 1);
642+
EXPECT_EQ(Map[K2], 2);
643+
EXPECT_EQ(Map[K3], 3);
644+
Map.clear();
645+
EXPECT_EQ(Map.find(K1), Map.end());
646+
EXPECT_EQ(Map.find(K2), Map.end());
647+
EXPECT_EQ(Map.find(K3), Map.end());
648+
}
625649
}

0 commit comments

Comments
 (0)