Skip to content

Commit

Permalink
Use accessors for ValueHandleBase::V; NFC (microsoft#6660)
Browse files Browse the repository at this point in the history
This PR pulls the upstream change, Use accessors for ValueHandleBase::V;
NFC
(llvm/llvm-project@6f08789),
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 microsoft#6659.

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
lizhengxing and github-actions[bot] authored Jun 6, 2024
1 parent 978d362 commit 8408ae8
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 32 deletions.
46 changes: 27 additions & 19 deletions include/llvm/IR/ValueHandle.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,48 +56,56 @@ class ValueHandleBase {
PointerIntPair<ValueHandleBase**, 2, HandleBaseKind> 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 &&
Expand Down
29 changes: 16 additions & 13 deletions lib/IR/Value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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?");
}
}

Expand All @@ -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;
Expand All @@ -595,10 +595,10 @@ void ValueHandleBase::AddToUseList() {
DenseMap<Value*, ValueHandleBase*> &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.
Expand All @@ -610,16 +610,19 @@ void ValueHandleBase::AddToUseList() {
// Okay, reallocation did happen. Fix the Prev Pointers.
for (DenseMap<Value*, ValueHandleBase*>::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");
Expand All @@ -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<Value*, ValueHandleBase*> &Handles = pImpl->ValueHandles;
if (Handles.isPointerIntoBucketsArray(PrevPtr)) {
Handles.erase(V);
V->HasValueHandle = false;
Handles.erase(getValPtr());
getValPtr()->HasValueHandle = false;
}
}

Expand Down

0 comments on commit 8408ae8

Please sign in to comment.