[llvm] [IR][ADT] Avoid isPointerIntoBucketsArray in RemoveFromUseList (NFC) (PR #228325)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 1 22:02:18 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Kazu Hirata (kazutakahirata)
<details>
<summary>Changes</summary>
RemoveFromUseList erases the entry from ValueHandles when removing the
sole remaining element in the doubly linked list. Without this patch,
after unlinking the tail element, it checks whether PrevPtr points into
the DenseMap bucket array:
if (Handles.isPointerIntoBucketsArray(PrevPtr)) {
Handles.erase(getValPtr());
This patch instead tags ValueHandleHead::Head with true via
PointerIntPair<ValueHandleBase *, 1, bool>, while leaving
ValueHandleBase::Next untagged. The tag bit allows RemoveFromUseList to
quickly determine whether it is removing the sole element in the list
without calling isPointerIntoBucketsArray.
Below is the breakdown of RemoveFromUseList calls and the perf stat -r10
comparison when compiling SLPVectorizer.ii with clang -O3:
Category Count %
---------------------------------------------------------
Non-tail element (Next != nullptr) 16,592,855 52.7%
Sole remaining element (erased) 8,198,144 26.0%
Tail of multi-element list (not erased) 6,714,742 21.3%
---------------------------------------------------------
Total 31,505,741 100.0%
Metric Base Test Delta
--------------------------------------------------------
instructions:u 194,745,673,762 194,513,892,029 -0.12%
cpu-cycles:u 141,184,939,025 141,042,112,760 -0.10%
time elapsed 54.9115s 54.8629s -0.09%
The instruction count decreases slightly because IsHead is readily
available from *PrevPtr. We do not need to chase
getValPtr()->getContext().pImpl or check bucket bounds when removing
the tail of a multi-element list.
With the final use of isPointerIntoBucketsArray gone, this patch removes
the following unused methods from DenseMapBase:
- isPointerIntoBucketsArray
- getPointerIntoBucketsArray
- getBucketsEnd
Assisted-by: Antigravity
---
Full diff: https://github.com/llvm/llvm-project/pull/228325.diff
4 Files Affected:
- (modified) llvm/include/llvm/ADT/DenseMap.h (-19)
- (modified) llvm/include/llvm/IR/ValueHandle.h (+1-1)
- (modified) llvm/lib/IR/LLVMContextImpl.h (+25-2)
- (modified) llvm/lib/IR/Value.cpp (+15-18)
``````````diff
diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index c4f6602fbbb02..126a588fdcfaa 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -990,19 +990,6 @@ class DenseMapBase : public DebugEpochBase {
return lookupOrInsertIntoBucket(std::move(Key)).first->second;
}
- /// Return true if the specified pointer points somewhere into the DenseMap's
- /// array of buckets (i.e. either to a key or value in the DenseMap).
- [[nodiscard]] bool isPointerIntoBucketsArray(const void *Ptr) const {
- return Ptr >= getBuckets() && Ptr < getBucketsEnd();
- }
-
- /// getPointerIntoBucketsArray() - Return an opaque pointer into the buckets
- /// array. In conjunction with the previous method, this can be used to
- /// determine whether an insertion caused the DenseMap to reallocate.
- [[nodiscard]] const void *getPointerIntoBucketsArray() const {
- return getBuckets();
- }
-
void swap(DenseMapBase &RHS) {
this->incrementEpoch();
RHS.incrementEpoch();
@@ -1252,12 +1239,6 @@ class DenseMapBase : public DebugEpochBase {
unsigned getNumBuckets() const { return Storage.getNumBuckets(); }
- BucketT *getBucketsEnd() { return getBuckets() + getNumBuckets(); }
-
- const BucketT *getBucketsEnd() const {
- return getBuckets() + getNumBuckets();
- }
-
LLVM_ATTRIBUTE_NOINLINE void grow(unsigned MinNumBuckets) {
assert((MinNumBuckets == 0 || isPowerOf2_32(MinNumBuckets)) &&
"bucket count must be zero or a power of two");
diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index b5b7947c8a8a3..153f68c0462a9 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -29,7 +29,7 @@ namespace llvm {
/// below for details.
class ValueHandleBase {
friend class Value;
- friend struct ValueHandleHead;
+ friend class ValueHandleHead;
template <typename ValueTy> friend class PoisoningVH;
protected:
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index 03ba382b88f42..7406c2c270ed0 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -1560,8 +1560,31 @@ struct MDAttachment {
/// Head pointer for a Value's ValueHandleBase doubly-linked list, stored in
/// LLVMContextImpl::ValueHandles. The first node's PrevPtr points to Head, so
/// relocating the bucket refreshes PrevPtr to the new Head address.
-struct ValueHandleHead {
- ValueHandleBase *Head = nullptr;
+class ValueHandleHead {
+ // Tag Head with true via PointerIntPair so RemoveFromUseList can
+ // distinguish ValueHandleHead::Head from an untagged ValueHandleBase::Next
+ // when accessed through *PrevPtr.
+ using TaggedPtr = PointerIntPair<ValueHandleBase *, 1, bool>;
+
+ ValueHandleBase *Head =
+ static_cast<ValueHandleBase *>(TaggedPtr(nullptr, true).getOpaqueValue());
+
+public:
+ ValueHandleBase *get() const {
+ return TaggedPtr::getFromOpaqueValue(Head).getPointer();
+ }
+
+ ValueHandleBase **getAddress() { return &Head; }
+
+ // Replace the pointer in *Slot with NewPtr while preserving its tag bit,
+ // and return the old TaggedPtr.
+ static TaggedPtr exchange(ValueHandleBase **Slot, ValueHandleBase *NewPtr) {
+ TaggedPtr Old = TaggedPtr::getFromOpaqueValue(*Slot);
+ TaggedPtr Updated = Old;
+ Updated.setPointer(NewPtr);
+ *Slot = static_cast<ValueHandleBase *>(Updated.getOpaqueValue());
+ return Old;
+ }
ValueHandleHead() = default;
ValueHandleHead(ValueHandleHead &&Other) noexcept;
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 0cef7f1838d38..f2bbf330477e4 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -1177,16 +1177,15 @@ bool Value::isSwiftError() const {
ValueHandleHead::ValueHandleHead(ValueHandleHead &&Other) noexcept
: Head(Other.Head) {
- if (Head)
- Head->setPrevPtr(&Head);
+ if (ValueHandleBase *Ptr = get())
+ Ptr->setPrevPtr(&Head);
}
void ValueHandleBase::AddToExistingUseList(ValueHandleBase **List) {
assert(List && "Handle list is null?");
// Splice ourselves into the list.
- Next = *List;
- *List = this;
+ Next = ValueHandleHead::exchange(List, this).getPointer();
setPrevPtr(List);
if (Next) {
Next->setPrevPtr(&Next);
@@ -1208,10 +1207,10 @@ void ValueHandleBase::AddToUseList() {
assert(getValPtr() && "Null pointer doesn't have a use list!");
LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl;
- ValueHandleBase *&Entry = pImpl->ValueHandles[getValPtr()].Head;
- assert(getValPtr()->hasValueHandle() == (Entry != nullptr) &&
+ ValueHandleHead &Entry = pImpl->ValueHandles[getValPtr()];
+ assert(getValPtr()->hasValueHandle() == (Entry.get() != nullptr) &&
"HasValueHandle and ValueHandles out of sync!");
- AddToExistingUseList(&Entry);
+ AddToExistingUseList(Entry.getAddress());
getValPtr()->HasValueHandle = true;
}
@@ -1221,9 +1220,8 @@ void ValueHandleBase::RemoveFromUseList() {
// Unlink this from its use list.
ValueHandleBase **PrevPtr = getPrevPtr();
- assert(*PrevPtr == this && "List invariant broken");
-
- *PrevPtr = Next;
+ auto [OldPtr, IsHead] = ValueHandleHead::exchange(PrevPtr, Next);
+ assert(OldPtr == this && "List invariant broken");
if (Next) {
assert(Next->getPrevPtr() == &Next && "List invariant broken");
Next->setPrevPtr(PrevPtr);
@@ -1233,10 +1231,9 @@ void ValueHandleBase::RemoveFromUseList() {
// If the Next pointer was null, then it is possible that this was the last
// ValueHandle watching VP. If so, delete its entry from the ValueHandles
// map.
- LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl;
- LLVMContextImpl::ValueHandlesTy &Handles = pImpl->ValueHandles;
- if (Handles.isPointerIntoBucketsArray(PrevPtr)) {
- Handles.erase(getValPtr());
+ if (IsHead) {
+ LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl;
+ pImpl->ValueHandles.erase(getValPtr());
getValPtr()->HasValueHandle = false;
}
}
@@ -1247,7 +1244,7 @@ void ValueHandleBase::ValueIsDeleted(Value *V) {
// Get the linked list base, which is guaranteed to exist since the
// HasValueHandle flag is set.
LLVMContextImpl *pImpl = V->getContext().pImpl;
- ValueHandleBase *Entry = pImpl->ValueHandles[V].Head;
+ ValueHandleBase *Entry = pImpl->ValueHandles[V].get();
assert(Entry && "Value bit set but no entries exist");
// We use a local ValueHandleBase as an iterator so that ValueHandles can add
@@ -1285,7 +1282,7 @@ void ValueHandleBase::ValueIsDeleted(Value *V) {
#ifndef NDEBUG // Only in +Asserts mode...
dbgs() << "While deleting: " << *V->getType() << " %" << V->getName()
<< "\n";
- if (pImpl->ValueHandles[V].Head->getKind() == Assert)
+ if (pImpl->ValueHandles[V].get()->getKind() == Assert)
llvm_unreachable("An asserting value handle still pointed to this"
" value!");
@@ -1303,7 +1300,7 @@ void ValueHandleBase::ValueIsRAUWd(Value *Old, Value *New) {
// Get the linked list base, which is guaranteed to exist since the
// HasValueHandle flag is set.
LLVMContextImpl *pImpl = Old->getContext().pImpl;
- ValueHandleBase *Entry = pImpl->ValueHandles[Old].Head;
+ ValueHandleBase *Entry = pImpl->ValueHandles[Old].get();
assert(Entry && "Value bit set but no entries exist");
@@ -1336,7 +1333,7 @@ void ValueHandleBase::ValueIsRAUWd(Value *Old, Value *New) {
// If any new weak value handles were added while processing the
// list, then complain about it now.
if (Old->HasValueHandle)
- for (Entry = pImpl->ValueHandles[Old].Head; Entry; Entry = Entry->Next)
+ for (Entry = pImpl->ValueHandles[Old].get(); Entry; Entry = Entry->Next)
switch (Entry->getKind()) {
case WeakTracking:
dbgs() << "After RAUW from " << *Old->getType() << " %"
``````````
</details>
https://github.com/llvm/llvm-project/pull/228325
More information about the llvm-commits
mailing list