[llvm] b2b7cf3 - IR: Clarify ownership of ConstantDataSequentials, NFC
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Mon Oct 26 15:47:48 PDT 2020
Author: Duncan P. N. Exon Smith
Date: 2020-10-26T18:47:25-04:00
New Revision: b2b7cf39d596b1528cd64015575b3f5d1461c011
URL: https://github.com/llvm/llvm-project/commit/b2b7cf39d596b1528cd64015575b3f5d1461c011
DIFF: https://github.com/llvm/llvm-project/commit/b2b7cf39d596b1528cd64015575b3f5d1461c011.diff
LOG: IR: Clarify ownership of ConstantDataSequentials, NFC
Change `ConstantDataSequential::Next` to a
`unique_ptr<ConstantDataSequential>` and update `CDSConstants` to a
`StringMap<unique_ptr<ConstantDataSequential>>`, making the ownership
more obvious.
Differential Revision: https://reviews.llvm.org/D90083
Added:
Modified:
llvm/include/llvm/IR/Constants.h
llvm/lib/IR/Constants.cpp
llvm/lib/IR/LLVMContextImpl.cpp
llvm/lib/IR/LLVMContextImpl.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index 343702aef03c..febcdb4d02c4 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -592,14 +592,13 @@ class ConstantDataSequential : public ConstantData {
/// the same value but
diff erent type. For example, 0,0,0,1 could be a 4
/// element array of i8, or a 1-element array of i32. They'll both end up in
/// the same StringMap bucket, linked up.
- ConstantDataSequential *Next;
+ std::unique_ptr<ConstantDataSequential> Next;
void destroyConstantImpl();
protected:
explicit ConstantDataSequential(Type *ty, ValueTy VT, const char *Data)
- : ConstantData(ty, VT), DataElements(Data), Next(nullptr) {}
- ~ConstantDataSequential() { delete Next; }
+ : ConstantData(ty, VT), DataElements(Data) {}
static Constant *getImpl(StringRef Bytes, Type *Ty);
diff --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 7eca7dddf4a4..cfefd338f784 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -2783,56 +2783,55 @@ Constant *ConstantDataSequential::getImpl(StringRef Elements, Type *Ty) {
// body but
diff erent types. For example, 0,0,0,1 could be a 4 element array
// of i8, or a 1-element array of i32. They'll both end up in the same
/// StringMap bucket, linked up by their Next pointers. Walk the list.
- ConstantDataSequential **Entry = &Slot.second;
- for (ConstantDataSequential *Node = *Entry; Node;
- Entry = &Node->Next, Node = *Entry)
+ std::unique_ptr<ConstantDataSequential> *Entry = &Slot.second;
+ for (ConstantDataSequential *Node = Entry->get(); Node;
+ Entry = &Node->Next, Node = Entry->get())
if (Node->getType() == Ty)
return Node;
// Okay, we didn't get a hit. Create a node of the right class, link it in,
// and return it.
- if (isa<ArrayType>(Ty))
- return *Entry = new ConstantDataArray(Ty, Slot.first().data());
+ if (isa<ArrayType>(Ty)) {
+ Entry->reset(new ConstantDataArray(Ty, Slot.first().data()));
+ return Entry->get();
+ }
assert(isa<VectorType>(Ty));
- return *Entry = new ConstantDataVector(Ty, Slot.first().data());
+ Entry->reset(new ConstantDataVector(Ty, Slot.first().data()));
+ return Entry->get();
}
void ConstantDataSequential::destroyConstantImpl() {
// Remove the constant from the StringMap.
- StringMap<ConstantDataSequential*> &CDSConstants =
- getType()->getContext().pImpl->CDSConstants;
+ StringMap<std::unique_ptr<ConstantDataSequential>> &CDSConstants =
+ getType()->getContext().pImpl->CDSConstants;
- StringMap<ConstantDataSequential*>::iterator Slot =
- CDSConstants.find(getRawDataValues());
+ auto Slot = CDSConstants.find(getRawDataValues());
assert(Slot != CDSConstants.end() && "CDS not found in uniquing table");
- ConstantDataSequential **Entry = &Slot->getValue();
+ std::unique_ptr<ConstantDataSequential> *Entry = &Slot->getValue();
// Remove the entry from the hash table.
if (!(*Entry)->Next) {
// If there is only one value in the bucket (common case) it must be this
// entry, and removing the entry should remove the bucket completely.
- assert((*Entry) == this && "Hash mismatch in ConstantDataSequential");
+ assert(Entry->get() == this && "Hash mismatch in ConstantDataSequential");
getContext().pImpl->CDSConstants.erase(Slot);
- } else {
- // Otherwise, there are multiple entries linked off the bucket, unlink the
- // node we care about but keep the bucket around.
- for (ConstantDataSequential *Node = *Entry; ;
- Entry = &Node->Next, Node = *Entry) {
- assert(Node && "Didn't find entry in its uniquing hash table!");
- // If we found our entry, unlink it from the list and we're done.
- if (Node == this) {
- *Entry = Node->Next;
- break;
- }
- }
+ return;
}
- // If we were part of a list, make sure that we don't delete the list that is
- // still owned by the uniquing map.
- Next = nullptr;
+ // Otherwise, there are multiple entries linked off the bucket, unlink the
+ // node we care about but keep the bucket around.
+ for (ConstantDataSequential *Node = Entry->get();;
+ Entry = &Node->Next, Node = Entry->get()) {
+ assert(Node && "Didn't find entry in its uniquing hash table!");
+ // If we found our entry, unlink it from the list and we're done.
+ if (Node == this) {
+ *Entry = std::move(Node->Next);
+ return;
+ }
+ }
}
/// getFP() constructors - Return a constant of array type with a float
diff --git a/llvm/lib/IR/LLVMContextImpl.cpp b/llvm/lib/IR/LLVMContextImpl.cpp
index 00f99715d24d..c4f0a0ac8549 100644
--- a/llvm/lib/IR/LLVMContextImpl.cpp
+++ b/llvm/lib/IR/LLVMContextImpl.cpp
@@ -99,9 +99,6 @@ LLVMContextImpl::~LLVMContextImpl() {
UVConstants.clear();
IntConstants.clear();
FPConstants.clear();
-
- for (auto &CDSConstant : CDSConstants)
- delete CDSConstant.second;
CDSConstants.clear();
// Destroy attribute node lists.
diff --git a/llvm/lib/IR/LLVMContextImpl.h b/llvm/lib/IR/LLVMContextImpl.h
index e676c5b378c7..86514ff8f194 100644
--- a/llvm/lib/IR/LLVMContextImpl.h
+++ b/llvm/lib/IR/LLVMContextImpl.h
@@ -1346,7 +1346,7 @@ class LLVMContextImpl {
DenseMap<Type *, std::unique_ptr<UndefValue>> UVConstants;
- StringMap<ConstantDataSequential*> CDSConstants;
+ StringMap<std::unique_ptr<ConstantDataSequential>> CDSConstants;
DenseMap<std::pair<const Function *, const BasicBlock *>, BlockAddress *>
BlockAddresses;
More information about the llvm-commits
mailing list