From 8408ae8829c9c5c5e5e5c01ae8603d45538f9339 Mon Sep 17 00:00:00 2001 From: Zhengxing li Date: Wed, 5 Jun 2024 23:20:46 -0700 Subject: [PATCH] Use accessors for ValueHandleBase::V; NFC (#6660) This PR pulls the upstream change, Use accessors for ValueHandleBase::V; NFC (https://github.com/llvm/llvm-project/commit/6f08789d3028d2ae46c15804c038d5d3f107a348), into DXC. Here's the summary of the change: > This changes code that touches ValueHandleBase::V to go through getValPtr and (newly added) setValPtr. This functionality will be used later, but also seemed like a generally good cleanup. > > I also renamed the field to Val, but that's just to make it obvious that I fixed all the uses. This is part 1 of the fix for #6659. --------- Co-authored-by: github-actions[bot] --- include/llvm/IR/ValueHandle.h | 46 ++++++++++++++++++++--------------- lib/IR/Value.cpp | 29 ++++++++++++---------- 2 files changed, 43 insertions(+), 32 deletions(-) diff --git a/include/llvm/IR/ValueHandle.h b/include/llvm/IR/ValueHandle.h index 7232bbfd08..08a59806a2 100644 --- a/include/llvm/IR/ValueHandle.h +++ b/include/llvm/IR/ValueHandle.h @@ -56,48 +56,56 @@ class ValueHandleBase { PointerIntPair PrevPair; ValueHandleBase *Next; - Value* V; + Value *Val; + + void setValPtr(Value *V) { Val = V; } ValueHandleBase(const ValueHandleBase&) = delete; public: explicit ValueHandleBase(HandleBaseKind Kind) - : PrevPair(nullptr, Kind), Next(nullptr), V(nullptr) {} + : PrevPair(nullptr, Kind), Next(nullptr), Val(nullptr) {} ValueHandleBase(HandleBaseKind Kind, Value *V) - : PrevPair(nullptr, Kind), Next(nullptr), V(V) { - if (isValid(V)) + : PrevPair(nullptr, Kind), Next(nullptr), Val(V) { + if (isValid(getValPtr())) AddToUseList(); } ValueHandleBase(HandleBaseKind Kind, const ValueHandleBase &RHS) - : PrevPair(nullptr, Kind), Next(nullptr), V(RHS.V) { - if (isValid(V)) + : PrevPair(nullptr, Kind), Next(nullptr), Val(RHS.getValPtr()) { + if (isValid(getValPtr())) AddToExistingUseList(RHS.getPrevPtr()); } ~ValueHandleBase() { - if (isValid(V)) + if (isValid(getValPtr())) RemoveFromUseList(); } Value *operator=(Value *RHS) { - if (V == RHS) return RHS; - if (isValid(V)) RemoveFromUseList(); - V = RHS; - if (isValid(V)) AddToUseList(); + if (getValPtr() == RHS) + return RHS; + if (isValid(getValPtr())) + RemoveFromUseList(); + setValPtr(RHS); + if (isValid(getValPtr())) + AddToUseList(); return RHS; } Value *operator=(const ValueHandleBase &RHS) { - if (V == RHS.V) return RHS.V; - if (isValid(V)) RemoveFromUseList(); - V = RHS.V; - if (isValid(V)) AddToExistingUseList(RHS.getPrevPtr()); - return V; + if (getValPtr() == RHS.getValPtr()) + return RHS.getValPtr(); + if (isValid(getValPtr())) + RemoveFromUseList(); + setValPtr(RHS.getValPtr()); + if (isValid(getValPtr())) + AddToExistingUseList(RHS.getPrevPtr()); + return getValPtr(); } - Value *operator->() const { return V; } - Value &operator*() const { return *V; } + Value *operator->() const { return getValPtr(); } + Value &operator*() const { return *getValPtr(); } protected: - Value *getValPtr() const { return V; } + Value *getValPtr() const { return Val; } static bool isValid(Value *V) { return V && diff --git a/lib/IR/Value.cpp b/lib/IR/Value.cpp index a9cfce28e0..0b07147694 100644 --- a/lib/IR/Value.cpp +++ b/lib/IR/Value.cpp @@ -559,7 +559,7 @@ void ValueHandleBase::AddToExistingUseList(ValueHandleBase **List) { setPrevPtr(List); if (Next) { Next->setPrevPtr(&Next); - assert(V == Next->V && "Added to wrong list?"); + assert(getValPtr() == Next->getValPtr() && "Added to wrong list?"); } } @@ -574,14 +574,14 @@ void ValueHandleBase::AddToExistingUseListAfter(ValueHandleBase *List) { } void ValueHandleBase::AddToUseList() { - assert(V && "Null pointer doesn't have a use list!"); + assert(getValPtr() && "Null pointer doesn't have a use list!"); - LLVMContextImpl *pImpl = V->getContext().pImpl; + LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl; - if (V->HasValueHandle) { + if (getValPtr()->HasValueHandle) { // If this value already has a ValueHandle, then it must be in the // ValueHandles map already. - ValueHandleBase *&Entry = pImpl->ValueHandles[V]; + ValueHandleBase *&Entry = pImpl->ValueHandles[getValPtr()]; assert(Entry && "Value doesn't have any handles?"); AddToExistingUseList(&Entry); return; @@ -595,10 +595,10 @@ void ValueHandleBase::AddToUseList() { DenseMap &Handles = pImpl->ValueHandles; const void *OldBucketPtr = Handles.getPointerIntoBucketsArray(); - ValueHandleBase *&Entry = Handles[V]; + ValueHandleBase *&Entry = Handles[getValPtr()]; assert(!Entry && "Value really did already have handles?"); AddToExistingUseList(&Entry); - V->HasValueHandle = true; + getValPtr()->HasValueHandle = true; // If reallocation didn't happen or if this was the first insertion, don't // walk the table. @@ -610,16 +610,19 @@ void ValueHandleBase::AddToUseList() { // Okay, reallocation did happen. Fix the Prev Pointers. for (DenseMap::iterator I = Handles.begin(), E = Handles.end(); I != E; ++I) { - assert(I->second && I->first == I->second->V && + assert(I->second && I->first == I->second->getValPtr() && "List invariant broken!"); I->second->setPrevPtr(&I->second); } } void ValueHandleBase::RemoveFromUseList() { - assert(V && (std::current_exception() == nullptr || V->HasValueHandle) && // HLSL Change + assert(getValPtr() && + (std::current_exception() == nullptr || + getValPtr()->HasValueHandle) && // HLSL Change "Pointer doesn't have a use list!"); - if (!V->HasValueHandle) return; // HLSL Change + if (!getValPtr()->HasValueHandle) + return; // HLSL Change // Unlink this from its use list. ValueHandleBase **PrevPtr = getPrevPtr(); assert(*PrevPtr == this && "List invariant broken"); @@ -634,11 +637,11 @@ 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 = V->getContext().pImpl; + LLVMContextImpl *pImpl = getValPtr()->getContext().pImpl; DenseMap &Handles = pImpl->ValueHandles; if (Handles.isPointerIntoBucketsArray(PrevPtr)) { - Handles.erase(V); - V->HasValueHandle = false; + Handles.erase(getValPtr()); + getValPtr()->HasValueHandle = false; } }