[llvm] [IR] Update ValueHandle PrevPtr via move constructor (NFC) (PR #227484)

Kazu Hirata via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 29 14:47:02 PDT 2026


https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/227484

In LLVMContextImpl::ValueHandles, the first ValueHandleBase node's
PrevPtr points directly to the head pointer inside the DenseMap
bucket, so relocating a bucket invalidates that PrevPtr.

This patch wraps the head pointer in ValueHandleHead, whose move
constructor updates Head->setPrevPtr(&Head) whenever a bucket is
relocated by grow() or erase().  Specifically, this allows us to:

- Simplify ValueHandleBase::AddToUseList by removing the manual
  reallocation check and linear fixup loop after insertion.

- Use the standard one-argument Handles.erase(getValPtr()) in
  ValueHandleBase::RemoveFromUseList instead of the private callback
  erase, addressing the TODO there.

A follow-up patch will remove the callback erase and friend class
ValueHandleBase from DenseMap.h.

Assisted-by: Antigravity


>From f66dcc236b4102ee20b0209fe24463211c35f2fa Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Tue, 29 Sep 2026 10:13:28 -0700
Subject: [PATCH] [IR] Update ValueHandle PrevPtr via move constructor (NFC)

In LLVMContextImpl::ValueHandles, the first ValueHandleBase node's
PrevPtr points directly to the head pointer inside the DenseMap
bucket, so relocating a bucket invalidates that PrevPtr.

This patch wraps the head pointer in ValueHandleHead, whose move
constructor updates Head->setPrevPtr(&Head) whenever a bucket is
relocated by grow() or erase().  Specifically, this allows us to:

- Simplify ValueHandleBase::AddToUseList by removing the manual
  reallocation check and linear fixup loop after insertion.

- Use the standard one-argument Handles.erase(getValPtr()) in
  ValueHandleBase::RemoveFromUseList instead of the private callback
  erase, addressing the TODO there.

A follow-up patch will remove the callback erase and friend class
ValueHandleBase from DenseMap.h.

Assisted-by: Antigravity
---
 llvm/include/llvm/IR/ValueHandle.h |  1 +
 llvm/lib/IR/LLVMContextImpl.h      | 15 +++++++-
 llvm/lib/IR/Value.cpp              | 58 ++++++++----------------------
 3 files changed, 30 insertions(+), 44 deletions(-)

diff --git a/llvm/include/llvm/IR/ValueHandle.h b/llvm/include/llvm/IR/ValueHandle.h
index 4ad034cb54646..b5b7947c8a8a3 100644
--- a/llvm/include/llvm/IR/ValueHandle.h
+++ b/llvm/include/llvm/IR/ValueHandle.h
@@ -29,6 +29,7 @@ namespace llvm {
 /// below for details.
 class ValueHandleBase {
   friend class Value;
+  friend struct ValueHandleHead;
   template <typename ValueTy> friend class PoisoningVH;
 
 protected:
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index ad733db864a8f..03ba382b88f42 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -1557,6 +1557,19 @@ struct MDAttachment {
   TrackingMDNodeRef Node;
 };
 
+/// 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;
+
+  ValueHandleHead() = default;
+  ValueHandleHead(ValueHandleHead &&Other) noexcept;
+  ValueHandleHead &operator=(ValueHandleHead &&) = delete;
+  ValueHandleHead(const ValueHandleHead &) = delete;
+  ValueHandleHead &operator=(const ValueHandleHead &) = delete;
+};
+
 class LLVMContextImpl {
 public:
   /// OwnedModules - The set of modules instantiated in this context, and which
@@ -1750,7 +1763,7 @@ class LLVMContextImpl {
   /// ValueHandles - This map keeps track of all of the value handles that are
   /// watching a Value*.  The Value::HasValueHandle bit is used to know
   /// whether or not a value has an entry in this map.
-  using ValueHandlesTy = DenseMap<Value *, ValueHandleBase *>;
+  using ValueHandlesTy = DenseMap<Value *, ValueHandleHead>;
   ValueHandlesTy ValueHandles;
 
   /// CustomMDKindNames - Map to hold the metadata string to ID mapping.
diff --git a/llvm/lib/IR/Value.cpp b/llvm/lib/IR/Value.cpp
index 2737192307b99..0cef7f1838d38 100644
--- a/llvm/lib/IR/Value.cpp
+++ b/llvm/lib/IR/Value.cpp
@@ -1175,6 +1175,12 @@ bool Value::isSwiftError() const {
 //                             ValueHandleBase Class
 //===----------------------------------------------------------------------===//
 
+ValueHandleHead::ValueHandleHead(ValueHandleHead &&Other) noexcept
+    : Head(Other.Head) {
+  if (Head)
+    Head->setPrevPtr(&Head);
+}
+
 void ValueHandleBase::AddToExistingUseList(ValueHandleBase **List) {
   assert(List && "Handle list is null?");
 
@@ -1202,42 +1208,11 @@ void ValueHandleBase::AddToUseList() {
   assert(getValPtr() && "Null pointer doesn't have a use list!");
 
   LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl;
-
-  if (getValPtr()->HasValueHandle) {
-    // If this value already has a ValueHandle, then it must be in the
-    // ValueHandles map already.
-    ValueHandleBase *&Entry = pImpl->ValueHandles[getValPtr()];
-    assert(Entry && "Value doesn't have any handles?");
-    AddToExistingUseList(&Entry);
-    return;
-  }
-
-  // Ok, it doesn't have any handles yet, so we must insert it into the
-  // DenseMap.  However, doing this insertion could cause the DenseMap to
-  // reallocate itself, which would invalidate all of the PrevP pointers that
-  // point into the old table.  Handle this by checking for reallocation and
-  // updating the stale pointers only if needed.
-  DenseMap<Value*, ValueHandleBase*> &Handles = pImpl->ValueHandles;
-  const void *OldBucketPtr = Handles.getPointerIntoBucketsArray();
-
-  ValueHandleBase *&Entry = Handles[getValPtr()];
-  assert(!Entry && "Value really did already have handles?");
+  ValueHandleBase *&Entry = pImpl->ValueHandles[getValPtr()].Head;
+  assert(getValPtr()->hasValueHandle() == (Entry != nullptr) &&
+         "HasValueHandle and ValueHandles out of sync!");
   AddToExistingUseList(&Entry);
   getValPtr()->HasValueHandle = true;
-
-  // If reallocation didn't happen or if this was the first insertion, don't
-  // walk the table.
-  if (Handles.isPointerIntoBucketsArray(OldBucketPtr) ||
-      Handles.size() == 1) {
-    return;
-  }
-
-  // Okay, reallocation did happen.  Fix the Prev Pointers.
-  for (auto I = Handles.begin(), E = Handles.end(); I != E; ++I) {
-    assert(I->second && I->first == I->second->getValPtr() &&
-           "List invariant broken!");
-    I->second->setPrevPtr(&I->second);
-  }
 }
 
 void ValueHandleBase::RemoveFromUseList() {
@@ -1259,12 +1234,9 @@ void ValueHandleBase::RemoveFromUseList() {
   // ValueHandle watching VP.  If so, delete its entry from the ValueHandles
   // map.
   LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl;
-  DenseMap<Value*, ValueHandleBase*> &Handles = pImpl->ValueHandles;
+  LLVMContextImpl::ValueHandlesTy &Handles = pImpl->ValueHandles;
   if (Handles.isPointerIntoBucketsArray(PrevPtr)) {
-    // TODO: Remove the only user of DenseMap's callback erase.
-    Handles.erase(getValPtr(), [](auto &Bucket) {
-      Bucket.second->setPrevPtr(&Bucket.second);
-    });
+    Handles.erase(getValPtr());
     getValPtr()->HasValueHandle = false;
   }
 }
@@ -1275,7 +1247,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];
+  ValueHandleBase *Entry = pImpl->ValueHandles[V].Head;
   assert(Entry && "Value bit set but no entries exist");
 
   // We use a local ValueHandleBase as an iterator so that ValueHandles can add
@@ -1313,7 +1285,7 @@ void ValueHandleBase::ValueIsDeleted(Value *V) {
 #ifndef NDEBUG      // Only in +Asserts mode...
     dbgs() << "While deleting: " << *V->getType() << " %" << V->getName()
            << "\n";
-    if (pImpl->ValueHandles[V]->getKind() == Assert)
+    if (pImpl->ValueHandles[V].Head->getKind() == Assert)
       llvm_unreachable("An asserting value handle still pointed to this"
                        " value!");
 
@@ -1331,7 +1303,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];
+  ValueHandleBase *Entry = pImpl->ValueHandles[Old].Head;
 
   assert(Entry && "Value bit set but no entries exist");
 
@@ -1364,7 +1336,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]; Entry; Entry = Entry->Next)
+    for (Entry = pImpl->ValueHandles[Old].Head; Entry; Entry = Entry->Next)
       switch (Entry->getKind()) {
       case WeakTracking:
         dbgs() << "After RAUW from " << *Old->getType() << " %"



More information about the llvm-commits mailing list