[Mlir-commits] [clang] [llvm] [mlir] [DenseMap] Do not align pointer sentinel values (NFC) (PR #146595)
Nikita Popov
llvmlistbot at llvm.org
Mon Jul 7 00:43:43 PDT 2025
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/146595
>From 28d67e3da912a0f6b09921874d1beb35a0548816 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Tue, 1 Jul 2025 13:51:43 +0200
Subject: [PATCH 1/3] [DenseMap] Do not align pointer sentinel values (NFC)
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 large immediates.
This patch proposed to use the values -1 and -2 instead, without
caring about pointer alignment. This puts us into unspecified
territory from the perspective of the C standard, but it is
Clang's explicit position that raw pointers do not carry alignment
guarantees (only accesses do).
We already have lots of places that do variations on
`reinterpret_cast<T*>(-1)` and assume that to work, so I'm not
sure it's worthwhile to be strictly standards conforming in this
single place.
---
llvm/include/llvm/ADT/DenseMapInfo.h | 25 ++++---------------------
llvm/include/llvm/ADT/PointerUnion.h | 14 ++++++++++----
2 files changed, 14 insertions(+), 25 deletions(-)
diff --git a/llvm/include/llvm/ADT/DenseMapInfo.h b/llvm/include/llvm/ADT/DenseMapInfo.h
index 07c37e353a40b..b371611e7d948 100644
--- a/llvm/include/llvm/ADT/DenseMapInfo.h
+++ b/llvm/include/llvm/ADT/DenseMapInfo.h
@@ -56,30 +56,13 @@ struct DenseMapInfo {
//static bool isEqual(const T &LHS, const T &RHS);
};
-// Provide DenseMapInfo for all pointers. Come up with sentinel pointer values
-// that are aligned to alignof(T) bytes, but try to avoid requiring T to be
-// complete. This allows clients to instantiate DenseMap<T*, ...> with forward
-// declared key types. Assume that no pointer key type requires more than 4096
-// bytes of alignment.
-template<typename T>
-struct DenseMapInfo<T*> {
- // The following should hold, but it would require T to be complete:
- // static_assert(alignof(T) <= (1 << Log2MaxAlign),
- // "DenseMap does not support pointer keys requiring more than "
- // "Log2MaxAlign bits of alignment");
- static constexpr uintptr_t Log2MaxAlign = 12;
-
+template <typename T> struct DenseMapInfo<T *> {
static inline T* getEmptyKey() {
- uintptr_t Val = static_cast<uintptr_t>(-1);
- Val <<= Log2MaxAlign;
- return reinterpret_cast<T*>(Val);
+ // We assume that raw pointers do not carry alignment requirements.
+ return reinterpret_cast<T *>(-1);
}
- static inline T* getTombstoneKey() {
- uintptr_t Val = static_cast<uintptr_t>(-2);
- Val <<= Log2MaxAlign;
- return reinterpret_cast<T*>(Val);
- }
+ static inline T *getTombstoneKey() { return reinterpret_cast<T *>(-2); }
static unsigned getHashValue(const T *PtrVal) {
return (unsigned((uintptr_t)PtrVal) >> 4) ^
diff --git a/llvm/include/llvm/ADT/PointerUnion.h b/llvm/include/llvm/ADT/PointerUnion.h
index cdbd76d7f505b..86248a2cf836f 100644
--- a/llvm/include/llvm/ADT/PointerUnion.h
+++ b/llvm/include/llvm/ADT/PointerUnion.h
@@ -286,13 +286,19 @@ struct PointerLikeTypeTraits<PointerUnion<PTs...>> {
// Teach DenseMap how to use PointerUnions as keys.
template <typename ...PTs> struct DenseMapInfo<PointerUnion<PTs...>> {
using Union = PointerUnion<PTs...>;
- using FirstInfo =
- DenseMapInfo<typename pointer_union_detail::GetFirstType<PTs...>::type>;
+ using FirstTypeTraits = PointerLikeTypeTraits<
+ typename pointer_union_detail::GetFirstType<PTs...>::type>;
- static inline Union getEmptyKey() { return Union(FirstInfo::getEmptyKey()); }
+ static inline Union getEmptyKey() {
+ uintptr_t Val = static_cast<uintptr_t>(-1);
+ Val <<= FirstTypeTraits::NumLowBitsAvailable;
+ return FirstTypeTraits::getFromVoidPointer(reinterpret_cast<void *>(Val));
+ }
static inline Union getTombstoneKey() {
- return Union(FirstInfo::getTombstoneKey());
+ uintptr_t Val = static_cast<uintptr_t>(-2);
+ Val <<= FirstTypeTraits::NumLowBitsAvailable;
+ return FirstTypeTraits::getFromVoidPointer(reinterpret_cast<void *>(Val));
}
static unsigned getHashValue(const Union &UnionVal) {
>From 28b0ceb084a0c6bba29232a3d2cae5dfd848a7dc Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 2 Jul 2025 09:53:39 +0200
Subject: [PATCH 2/3] Update DenseMapInfo for LValueBase
---
clang/lib/AST/APValue.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/clang/lib/AST/APValue.cpp b/clang/lib/AST/APValue.cpp
index c641ff6b99bab..acba55742a967 100644
--- a/clang/lib/AST/APValue.cpp
+++ b/clang/lib/AST/APValue.cpp
@@ -187,14 +187,14 @@ APValue::LValueBase::operator bool () const {
clang::APValue::LValueBase
llvm::DenseMapInfo<clang::APValue::LValueBase>::getEmptyKey() {
clang::APValue::LValueBase B;
- B.Ptr = DenseMapInfo<const ValueDecl*>::getEmptyKey();
+ B.Ptr = DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getEmptyKey();
return B;
}
clang::APValue::LValueBase
llvm::DenseMapInfo<clang::APValue::LValueBase>::getTombstoneKey() {
clang::APValue::LValueBase B;
- B.Ptr = DenseMapInfo<const ValueDecl*>::getTombstoneKey();
+ B.Ptr = DenseMapInfo<clang::APValue::LValueBase::PtrTy>::getTombstoneKey();
return B;
}
>From a7d8fe34b57939a77502da1f2667a19374c19d0d Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Wed, 2 Jul 2025 15:39:10 +0200
Subject: [PATCH 3/3] Adjust TypeID DenseMapInfo
---
mlir/include/mlir/Support/TypeID.h | 5 +++--
mlir/lib/Bindings/Python/NanobindUtils.h | 5 +++--
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/mlir/include/mlir/Support/TypeID.h b/mlir/include/mlir/Support/TypeID.h
index 459e9dae12a9f..e2c25474087ae 100644
--- a/mlir/include/mlir/Support/TypeID.h
+++ b/mlir/include/mlir/Support/TypeID.h
@@ -395,11 +395,12 @@ namespace llvm {
template <>
struct DenseMapInfo<mlir::TypeID> {
static inline mlir::TypeID getEmptyKey() {
- void *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
+ // Shift by 3 to satisfy the TypeID alignment requirement.
+ void *pointer = reinterpret_cast<void *>(uintptr_t(-1) << 3);
return mlir::TypeID::getFromOpaquePointer(pointer);
}
static inline mlir::TypeID getTombstoneKey() {
- void *pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
+ void *pointer = reinterpret_cast<void *>(uintptr_t(-2) << 3);
return mlir::TypeID::getFromOpaquePointer(pointer);
}
static unsigned getHashValue(mlir::TypeID val) {
diff --git a/mlir/lib/Bindings/Python/NanobindUtils.h b/mlir/lib/Bindings/Python/NanobindUtils.h
index 64ea4329f65f1..535fc2328c482 100644
--- a/mlir/lib/Bindings/Python/NanobindUtils.h
+++ b/mlir/lib/Bindings/Python/NanobindUtils.h
@@ -408,11 +408,12 @@ namespace llvm {
template <>
struct DenseMapInfo<MlirTypeID> {
static inline MlirTypeID getEmptyKey() {
- auto *pointer = llvm::DenseMapInfo<void *>::getEmptyKey();
+ // Shift by 3 to satisfy the TypeID alignment requirement.
+ void *pointer = reinterpret_cast<void *>(uintptr_t(-1) << 3);
return mlirTypeIDCreate(pointer);
}
static inline MlirTypeID getTombstoneKey() {
- auto *pointer = llvm::DenseMapInfo<void *>::getTombstoneKey();
+ void *pointer = reinterpret_cast<void *>(uintptr_t(-2) << 3);
return mlirTypeIDCreate(pointer);
}
static inline unsigned getHashValue(const MlirTypeID &val) {
More information about the Mlir-commits
mailing list