[llvm] r301809 - Emulate TrackingVH using WeakVH
Sanjoy Das via llvm-commits
llvm-commits at lists.llvm.org
Mon May 1 09:28:59 PDT 2017
Author: sanjoy
Date: Mon May 1 11:28:58 2017
New Revision: 301809
URL: http://llvm.org/viewvc/llvm-project?rev=301809&view=rev
Log:
Emulate TrackingVH using WeakVH
Summary:
This frees up one slot in the HandleBaseKind enum, which I will use
later to add a new kind of value handle. The size of the
HandleBaseKind enum is important because we store a HandleBaseKind in
the low two bits of a (in the worst case) 4 byte aligned pointer.
Reviewers: davide, chandlerc
Subscribers: mcrosier, llvm-commits
Differential Revision: https://reviews.llvm.org/D32634
Modified:
llvm/trunk/include/llvm/IR/ValueHandle.h
llvm/trunk/lib/IR/Value.cpp
llvm/trunk/unittests/IR/ValueHandleTest.cpp
Modified: llvm/trunk/include/llvm/IR/ValueHandle.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/ValueHandle.h?rev=301809&r1=301808&r2=301809&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/ValueHandle.h (original)
+++ llvm/trunk/include/llvm/IR/ValueHandle.h Mon May 1 11:28:58 2017
@@ -37,7 +37,6 @@ protected:
enum HandleBaseKind {
Assert,
Callback,
- Tracking,
Weak
};
@@ -165,6 +164,10 @@ public:
operator Value*() const {
return getValPtr();
}
+
+ bool pointsToAliveValue() const {
+ return ValueHandleBase::isValid(getValPtr());
+ }
};
// Specialize simplify_type to allow WeakVH to participate in
@@ -280,39 +283,37 @@ struct isPodLike<AssertingVH<T> > {
/// to a Value (or subclass) across some operations which may move that value,
/// but should never destroy it or replace it with some unacceptable type.
///
-/// It is an error to do anything with a TrackingVH whose value has been
-/// destroyed, except to destruct it.
-///
/// It is an error to attempt to replace a value with one of a type which is
/// incompatible with any of its outstanding TrackingVHs.
-template<typename ValueTy>
-class TrackingVH : public ValueHandleBase {
- void CheckValidity() const {
- Value *VP = ValueHandleBase::getValPtr();
-
- // Null is always ok.
- if (!VP) return;
-
- // Check that this value is valid (i.e., it hasn't been deleted). We
- // explicitly delay this check until access to avoid requiring clients to be
- // unnecessarily careful w.r.t. destruction.
- assert(ValueHandleBase::isValid(VP) && "Tracked Value was deleted!");
+///
+/// It is an error to read from a TrackingVH that does not point to a valid
+/// value. A TrackingVH is said to not point to a valid value if either it
+/// hasn't yet been assigned a value yet or because the value it was tracking
+/// has since been deleted.
+///
+/// Assigning a value to a TrackingVH is always allowed, even if said TrackingVH
+/// no longer points to a valid value.
+template <typename ValueTy> class TrackingVH {
+ WeakVH InnerHandle;
+
+public:
+ ValueTy *getValPtr() const {
+ assert(InnerHandle.pointsToAliveValue() &&
+ "TrackingVH must be non-null and valid on dereference!");
// Check that the value is a member of the correct subclass. We would like
// to check this property on assignment for better debugging, but we don't
// want to require a virtual interface on this VH. Instead we allow RAUW to
// replace this value with a value of an invalid type, and check it here.
- assert(isa<ValueTy>(VP) &&
+ assert(isa<ValueTy>(InnerHandle) &&
"Tracked Value was replaced by one with an invalid type!");
+ return cast<ValueTy>(InnerHandle);
}
- ValueTy *getValPtr() const {
- CheckValidity();
- return (ValueTy*)ValueHandleBase::getValPtr();
- }
void setValPtr(ValueTy *P) {
- CheckValidity();
- ValueHandleBase::operator=(GetAsValue(P));
+ // Assigning to non-valid TrackingVH's are fine so we just unconditionally
+ // assign here.
+ InnerHandle = GetAsValue(P);
}
// Convert a ValueTy*, which may be const, to the type the base
@@ -321,8 +322,8 @@ class TrackingVH : public ValueHandleBas
static Value *GetAsValue(const Value *V) { return const_cast<Value*>(V); }
public:
- TrackingVH() : ValueHandleBase(Tracking) {}
- TrackingVH(ValueTy *P) : ValueHandleBase(Tracking, GetAsValue(P)) {}
+ TrackingVH() {}
+ TrackingVH(ValueTy *P) { setValPtr(P); }
operator ValueTy*() const {
return getValPtr();
Modified: llvm/trunk/lib/IR/Value.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Value.cpp?rev=301809&r1=301808&r2=301809&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Value.cpp (original)
+++ llvm/trunk/lib/IR/Value.cpp Mon May 1 11:28:58 2017
@@ -820,11 +820,6 @@ void ValueHandleBase::ValueIsDeleted(Val
switch (Entry->getKind()) {
case Assert:
break;
- case Tracking:
- // Mark that this value has been deleted by setting it to an invalid Value
- // pointer.
- Entry->operator=(DenseMapInfo<Value *>::getTombstoneKey());
- break;
case Weak:
// Weak just goes to null, which will unlink it from the list.
Entry->operator=(nullptr);
@@ -876,13 +871,6 @@ void ValueHandleBase::ValueIsRAUWd(Value
case Assert:
// Asserting handle does not follow RAUW implicitly.
break;
- case Tracking:
- // Tracking goes to new value like a WeakVH. Note that this may make it
- // something incompatible with its templated type. We don't want to have a
- // virtual (or inline) interface to handle this though, so instead we make
- // the TrackingVH accessors guarantee that a client never sees this value.
-
- LLVM_FALLTHROUGH;
case Weak:
// Weak goes to the new value, which will unlink it from Old's list.
Entry->operator=(New);
@@ -895,18 +883,17 @@ void ValueHandleBase::ValueIsRAUWd(Value
}
#ifndef NDEBUG
- // If any new tracking or weak value handles were added while processing the
+ // 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)
switch (Entry->getKind()) {
- case Tracking:
case Weak:
dbgs() << "After RAUW from " << *Old->getType() << " %"
<< Old->getName() << " to " << *New->getType() << " %"
<< New->getName() << "\n";
- llvm_unreachable("A tracking or weak value handle still pointed to the"
- " old value!\n");
+ llvm_unreachable(
+ "A weak value handle still pointed to the old value!\n");
default:
break;
}
Modified: llvm/trunk/unittests/IR/ValueHandleTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/ValueHandleTest.cpp?rev=301809&r1=301808&r2=301809&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/ValueHandleTest.cpp (original)
+++ llvm/trunk/unittests/IR/ValueHandleTest.cpp Mon May 1 11:28:58 2017
@@ -482,6 +482,12 @@ TEST_F(ValueHandle, PoisoningVH_ReducesT
#else // !NDEBUG
+TEST_F(ValueHandle, TrackingVH_Tracks) {
+ TrackingVH<Value> VH(BitcastV.get());
+ BitcastV->replaceAllUsesWith(ConstantV);
+ EXPECT_EQ(VH, ConstantV);
+}
+
#ifdef GTEST_HAS_DEATH_TEST
TEST_F(ValueHandle, PoisoningVH_Asserts) {
@@ -502,6 +508,26 @@ TEST_F(ValueHandle, PoisoningVH_Asserts)
// Don't clear anything out here as destroying the handles should be fine.
}
+TEST_F(ValueHandle, TrackingVH_Asserts) {
+ {
+ TrackingVH<Value> VH(BitcastV.get());
+
+ // The tracking handle shouldn't assert when the value is deleted.
+ BitcastV.reset(new BitCastInst(ConstantV, Type::getInt32Ty(Context)));
+ // But should when we access the handle.
+ EXPECT_DEATH((void)*VH,
+ "TrackingVH must be non-null and valid on dereference!");
+ }
+
+ {
+ TrackingVH<Instruction> VH(BitcastV.get());
+
+ BitcastV->replaceAllUsesWith(ConstantV);
+ EXPECT_DEATH((void)*VH,
+ "Tracked Value was replaced by one with an invalid type!");
+ }
+}
+
#endif // GTEST_HAS_DEATH_TEST
#endif // NDEBUG
More information about the llvm-commits
mailing list