Skip to content

Commit 7a6435b

Browse files
authored
[DenseMap] Do not align pointer sentinel values (NFC) (#146595)
DenseMapInfo for pointers currently uses empty/tombstone values that are aligned (by assuming a very conservative alignment). However, this means that we have to work with larger immediates. This patch proposes to use the values -1 and -2 instead, without caring about pointer alignment. (Non-roundtrip) integer to pointer casts are implementation-defined in C++, but the general implementer consensus (including Clang) is that raw pointers do not carry alignment requirements, only memory accesses do. We already have lots of places that rely on this using variations on `reinterpret_cast<T*>(-1)`, so it seems odd to insist on properly aligned pointers in this one place. It is necessary to adjust a few other places after this change, which currently assume that `DenseMapInfo<void *>` returns a highly-aligned pointer. This is a small improvement for both compile-time and clang binary size.
1 parent c19c71b commit 7a6435b

File tree

5 files changed

+22
-31
lines changed

5 files changed

+22
-31
lines changed

clang/lib/AST/APValue.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,14 @@ APValue::LValueBase::operator bool () const {
187187
clang::APValue::LValueBase
188188
llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() {
189189
clang::APValue::LValueBase B;
190-
B.Ptr = DenseMapInfo<const ValueDecl*>::getEmptyKey();
190+
B.Ptr = DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getEmptyKey();
191191
return B;
192192
}
193193

194194
clang::APValue::LValueBase
195195
llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() {
196196
clang::APValue::LValueBase B;
197-
B.Ptr = DenseMapInfo<const ValueDecl*>::getTombstoneKey();
197+
B.Ptr = DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getTombstoneKey();
198198
return B;
199199
}
200200

llvm/include/llvm/ADT/DenseMapInfo.h

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -56,30 +56,13 @@ struct DenseMapInfo {
5656
//static bool isEqual(const T &LHS, const T &RHS);
5757
};
5858

59-
// Provide DenseMapInfo for all pointers. Come up with sentinel pointer values
60-
// that are aligned to alignof(T) bytes, but try to avoid requiring T to be
61-
// complete. This allows clients to instantiate DenseMap<T*, ...> with forward
62-
// declared key types. Assume that no pointer key type requires more than 4096
63-
// bytes of alignment.
64-
template<typename T>
65-
struct DenseMapInfo<T*> {
66-
// The following should hold, but it would require T to be complete:
67-
// static_assert(alignof(T) <= (1 << Log2MaxAlign),
68-
// "DenseMap does not support pointer keys requiring more than "
69-
// "Log2MaxAlign bits of alignment");
70-
static constexpr uintptr_t Log2MaxAlign = 12;
71-
59+
template <typename T> struct DenseMapInfo<T *> {
7260
static inline T* getEmptyKey() {
73-
uintptr_t Val = static_cast<uintptr_t>(-1);
74-
Val <<= Log2MaxAlign;
75-
return reinterpret_cast<T*>(Val);
61+
// We assume that raw pointers do not carry alignment requirements.
62+
return reinterpret_cast<T *>(-1);
7663
}
7764

78-
static inline T* getTombstoneKey() {
79-
uintptr_t Val = static_cast<uintptr_t>(-2);
80-
Val <<= Log2MaxAlign;
81-
return reinterpret_cast<T*>(Val);
82-
}
65+
static inline T *getTombstoneKey() { return reinterpret_cast<T *>(-2); }
8366

8467
static unsigned getHashValue(const T *PtrVal) {
8568
return (unsigned((uintptr_t)PtrVal) >> 4) ^

llvm/include/llvm/ADT/PointerUnion.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -286,13 +286,19 @@ struct PointerLikeTypeTraits<PointerUnion<PTs...>> {
286286
// Teach DenseMap how to use PointerUnions as keys.
287287
template <typename ...PTs> struct DenseMapInfo<PointerUnion<PTs...>> {
288288
using Union = PointerUnion<PTs...>;
289-
using FirstInfo =
290-
DenseMapInfo<typename pointer_union_detail::GetFirstType<PTs...>::type>;
289+
using FirstTypeTraits = PointerLikeTypeTraits<
290+
typename pointer_union_detail::GetFirstType<PTs...>::type>;
291291

292-
static inline Union getEmptyKey() { return Union(FirstInfo::getEmptyKey()); }
292+
static inline Union getEmptyKey() {
293+
uintptr_t Val = static_cast<uintptr_t>(-1);
294+
Val <<= FirstTypeTraits::NumLowBitsAvailable;
295+
return FirstTypeTraits::getFromVoidPointer(reinterpret_cast<void *>(Val));
296+
}
293297

294298
static inline Union getTombstoneKey() {
295-
return Union(FirstInfo::getTombstoneKey());
299+
uintptr_t Val = static_cast<uintptr_t>(-2);
300+
Val <<= FirstTypeTraits::NumLowBitsAvailable;
301+
return FirstTypeTraits::getFromVoidPointer(reinterpret_cast<void *>(Val));
296302
}
297303

298304
static unsigned getHashValue(const Union &UnionVal) {

mlir/include/mlir/Support/TypeID.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -395,11 +395,12 @@ namespace llvm {
395395
template <>
396396
struct DenseMapInfo<mlir::TypeID> {
397397
static inline mlir::TypeID getEmptyKey() {
398-
void *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
398+
// Shift by 3 to satisfy the TypeID alignment requirement.
399+
void *pointer = reinterpret_cast<void *>(uintptr_t(-1) << 3);
399400
return mlir::TypeID::getFromOpaquePointer(pointer);
400401
}
401402
static inline mlir::TypeID getTombstoneKey() {
402-
void *pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
403+
void *pointer = reinterpret_cast<void *>(uintptr_t(-2) << 3);
403404
return mlir::TypeID::getFromOpaquePointer(pointer);
404405
}
405406
static unsigned getHashValue(mlir::TypeID val) {

mlir/lib/Bindings/Python/NanobindUtils.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,11 +408,12 @@ namespace llvm {
408408
template <>
409409
struct DenseMapInfo<MlirTypeID> {
410410
static inline MlirTypeID getEmptyKey() {
411-
auto *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
411+
// Shift by 3 to satisfy the TypeID alignment requirement.
412+
void *pointer = reinterpret_cast<void *>(uintptr_t(-1) << 3);
412413
return mlirTypeIDCreate(pointer);
413414
}
414415
static inline MlirTypeID getTombstoneKey() {
415-
auto *pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
416+
void *pointer = reinterpret_cast<void *>(uintptr_t(-2) << 3);
416417
return mlirTypeIDCreate(pointer);
417418
}
418419
static inline unsigned getHashValue(const MlirTypeID &val) {

0 commit comments

Comments
 (0)