From f9eaea701dd2771fbd72145ed8eaaaeb62779b08 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 19 Jan 2015 21:30:18 +0000 Subject: [PATCH] IR: Return unique_ptr from MDNode::getTemporary() Change `MDTuple::getTemporary()` and `MDLocation::getTemporary()` to return (effectively) `std::unique_ptr`, and clean up call sites. (For now, `DIBuilder` call sites just call `release()` immediately.) There's an accompanying change in each of clang and polly to use the new API. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226504 91177308-0d34-0410-b5e6-96231b3b80d8 --- bindings/go/llvm/IRBindings.cpp | 5 ++- include/llvm/IR/Metadata.h | 34 +++++++++++----- lib/AsmParser/LLParser.cpp | 12 +++--- lib/AsmParser/LLParser.h | 2 +- lib/Bitcode/Reader/BitcodeReader.cpp | 5 +-- lib/IR/DIBuilder.cpp | 18 ++++----- lib/IR/MDBuilder.cpp | 6 +-- lib/Transforms/Utils/InlineFunction.cpp | 11 ++--- lib/Transforms/Utils/ValueMapper.cpp | 11 ++--- unittests/IR/MetadataTest.cpp | 54 ++++++++++--------------- 10 files changed, 77 insertions(+), 81 deletions(-) diff --git a/bindings/go/llvm/IRBindings.cpp b/bindings/go/llvm/IRBindings.cpp index 7c2fe60b2b9..402407eaf8d 100644 --- a/bindings/go/llvm/IRBindings.cpp +++ b/bindings/go/llvm/IRBindings.cpp @@ -66,8 +66,9 @@ LLVMMetadataRef LLVMMDNode2(LLVMContextRef C, LLVMMetadataRef *MDs, LLVMMetadataRef LLVMTemporaryMDNode(LLVMContextRef C, LLVMMetadataRef *MDs, unsigned Count) { - return wrap(MDNode::getTemporary(*unwrap(C), - ArrayRef(unwrap(MDs), Count))); + return wrap(MDTuple::getTemporary(*unwrap(C), + ArrayRef(unwrap(MDs), Count)) + .release()); } void LLVMAddNamedMetadataOperand2(LLVMModuleRef M, const char *name, diff --git a/include/llvm/IR/Metadata.h b/include/llvm/IR/Metadata.h index 2b37412660d..a9e54fd84c9 100644 --- a/include/llvm/IR/Metadata.h +++ b/include/llvm/IR/Metadata.h @@ -651,6 +651,15 @@ public: } }; +template +struct TempMDNodeDeleter { + inline void operator()(T *Node) const; +}; + +#define HANDLE_UNIQUABLE_LEAF(CLASS) \ + typedef std::unique_ptr> Temp##CLASS; +#include "llvm/IR/Metadata.def" + //===----------------------------------------------------------------------===// /// \brief Tuple of metadata. class MDNode : public Metadata { @@ -697,8 +706,8 @@ public: ArrayRef MDs); static inline MDTuple *getDistinct(LLVMContext &Context, ArrayRef MDs); - static inline MDTuple *getTemporary(LLVMContext &Context, - ArrayRef MDs); + static inline TempMDTuple getTemporary(LLVMContext &Context, + ArrayRef MDs); /// \brief Deallocate a node created by getTemporary. /// @@ -884,8 +893,9 @@ public: /// For use in constructing cyclic MDNode structures. A temporary MDNode is /// not uniqued, may be RAUW'd, and must be manually deleted with /// deleteTemporary. - static MDTuple *getTemporary(LLVMContext &Context, ArrayRef MDs) { - return getImpl(Context, MDs, Temporary); + static TempMDTuple getTemporary(LLVMContext &Context, + ArrayRef MDs) { + return TempMDTuple(getImpl(Context, MDs, Temporary)); } static bool classof(const Metadata *MD) { @@ -906,9 +916,14 @@ MDTuple *MDNode::getIfExists(LLVMContext &Context, ArrayRef MDs) { MDTuple *MDNode::getDistinct(LLVMContext &Context, ArrayRef MDs) { return MDTuple::getDistinct(Context, MDs); } -MDTuple *MDNode::getTemporary(LLVMContext &Context, ArrayRef MDs) { +TempMDTuple MDNode::getTemporary(LLVMContext &Context, + ArrayRef MDs) { return MDTuple::getTemporary(Context, MDs); } +template +void TempMDNodeDeleter::operator()(T *Node) const { + MDNode::deleteTemporary(Node); +} /// \brief Debug location. /// @@ -945,10 +960,11 @@ public: Metadata *InlinedAt = nullptr) { return getImpl(Context, Line, Column, Scope, InlinedAt, Distinct); } - static MDLocation *getTemporary(LLVMContext &Context, unsigned Line, - unsigned Column, Metadata *Scope, - Metadata *InlinedAt = nullptr) { - return getImpl(Context, Line, Column, Scope, InlinedAt, Temporary); + static TempMDLocation getTemporary(LLVMContext &Context, unsigned Line, + unsigned Column, Metadata *Scope, + Metadata *InlinedAt = nullptr) { + return TempMDLocation( + getImpl(Context, Line, Column, Scope, InlinedAt, Temporary)); } unsigned getLine() const { return MDNodeSubclassData; } diff --git a/lib/AsmParser/LLParser.cpp b/lib/AsmParser/LLParser.cpp index ce195381cfb..421a2bcbaba 100644 --- a/lib/AsmParser/LLParser.cpp +++ b/lib/AsmParser/LLParser.cpp @@ -531,13 +531,13 @@ bool LLParser::ParseMDNodeID(MDNode *&Result) { } // Otherwise, create MDNode forward reference. - MDTuple *FwdNode = MDTuple::getTemporary(Context, None); - ForwardRefMDNodes[MID] = std::make_pair(FwdNode, Lex.getLoc()); + auto &FwdRef = ForwardRefMDNodes[MID]; + FwdRef = std::make_pair(MDTuple::getTemporary(Context, None), Lex.getLoc()); if (NumberedMetadata.size() <= MID) NumberedMetadata.resize(MID+1); - NumberedMetadata[MID].reset(FwdNode); - Result = FwdNode; + Result = FwdRef.first.get(); + NumberedMetadata[MID].reset(Result); return false; } @@ -597,9 +597,7 @@ bool LLParser::ParseStandaloneMetadata() { // See if this was forward referenced, if so, handle it. auto FI = ForwardRefMDNodes.find(MetadataID); if (FI != ForwardRefMDNodes.end()) { - MDTuple *Temp = FI->second.first; - Temp->replaceAllUsesWith(Init); - MDNode::deleteTemporary(Temp); + FI->second.first->replaceAllUsesWith(Init); ForwardRefMDNodes.erase(FI); assert(NumberedMetadata[MetadataID] == Init && "Tracking VH didn't work"); diff --git a/lib/AsmParser/LLParser.h b/lib/AsmParser/LLParser.h index 54e0d5bf020..b1f94c237a9 100644 --- a/lib/AsmParser/LLParser.h +++ b/lib/AsmParser/LLParser.h @@ -136,7 +136,7 @@ namespace llvm { std::vector > NumberedTypes; std::vector NumberedMetadata; - std::map> ForwardRefMDNodes; + std::map> ForwardRefMDNodes; // Global Value reference information. std::map > ForwardRefVals; diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp index a9adaf4febe..c158d356a16 100644 --- a/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/lib/Bitcode/Reader/BitcodeReader.cpp @@ -541,9 +541,8 @@ void BitcodeReaderMDValueList::AssignValue(Metadata *MD, unsigned Idx) { } // If there was a forward reference to this value, replace it. - MDTuple *PrevMD = cast(OldMD.get()); + TempMDTuple PrevMD(cast(OldMD.get())); PrevMD->replaceAllUsesWith(MD); - MDNode::deleteTemporary(PrevMD); --NumFwdRefs; } @@ -557,7 +556,7 @@ Metadata *BitcodeReaderMDValueList::getValueFwdRef(unsigned Idx) { // Create and return a placeholder, which will later be RAUW'd. AnyFwdRefs = true; ++NumFwdRefs; - Metadata *MD = MDNode::getTemporary(Context, None); + Metadata *MD = MDNode::getTemporary(Context, None).release(); MDValuePtrs[Idx].reset(MD); return MD; } diff --git a/lib/IR/DIBuilder.cpp b/lib/IR/DIBuilder.cpp index 2de109b86cf..fdc0b1bb97b 100644 --- a/lib/IR/DIBuilder.cpp +++ b/lib/IR/DIBuilder.cpp @@ -142,15 +142,15 @@ DICompileUnit DIBuilder::createCompileUnit(unsigned Lang, StringRef Filename, assert(!Filename.empty() && "Unable to create compile unit without filename"); Metadata *TElts[] = {HeaderBuilder::get(DW_TAG_base_type).get(VMContext)}; - TempEnumTypes = MDNode::getTemporary(VMContext, TElts); + TempEnumTypes = MDNode::getTemporary(VMContext, TElts).release(); - TempRetainTypes = MDNode::getTemporary(VMContext, TElts); + TempRetainTypes = MDNode::getTemporary(VMContext, TElts).release(); - TempSubprograms = MDNode::getTemporary(VMContext, TElts); + TempSubprograms = MDNode::getTemporary(VMContext, TElts).release(); - TempGVs = MDNode::getTemporary(VMContext, TElts); + TempGVs = MDNode::getTemporary(VMContext, TElts).release(); - TempImportedModules = MDNode::getTemporary(VMContext, TElts); + TempImportedModules = MDNode::getTemporary(VMContext, TElts).release(); Metadata *Elts[] = {HeaderBuilder::get(dwarf::DW_TAG_compile_unit) .concat(Lang) @@ -825,7 +825,7 @@ DICompositeType DIBuilder::createReplaceableForwardDecl( nullptr, // TemplateParams UniqueIdentifier.empty() ? nullptr : MDString::get(VMContext, UniqueIdentifier)}; - DICompositeType RetTy(MDNode::getTemporary(VMContext, Elts)); + DICompositeType RetTy(MDNode::getTemporary(VMContext, Elts).release()); assert(RetTy.isCompositeType() && "createReplaceableForwardDecl result should be a DIType"); if (!UniqueIdentifier.empty()) @@ -903,7 +903,7 @@ DIGlobalVariable DIBuilder::createTempGlobalVariableFwdDecl( return createGlobalVariableHelper(VMContext, Context, Name, LinkageName, F, LineNumber, Ty, isLocalToUnit, Val, Decl, false, [&](ArrayRef Elts) { - return MDNode::getTemporary(VMContext, Elts); + return MDNode::getTemporary(VMContext, Elts).release(); }); } @@ -1007,7 +1007,7 @@ DISubprogram DIBuilder::createFunction(DIDescriptor Context, StringRef Name, return createFunctionHelper(VMContext, Context, Name, LinkageName, File, LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine, Flags, isOptimized, Fn, TParams, Decl, - MDNode::getTemporary(VMContext, None), + MDNode::getTemporary(VMContext, None).release(), [&](ArrayRef Elts) -> MDNode *{ MDNode *Node = MDNode::get(VMContext, Elts); // Create a named metadata so that we @@ -1030,7 +1030,7 @@ DIBuilder::createTempFunctionFwdDecl(DIDescriptor Context, StringRef Name, LineNo, Ty, isLocalToUnit, isDefinition, ScopeLine, Flags, isOptimized, Fn, TParams, Decl, nullptr, [&](ArrayRef Elts) { - return MDNode::getTemporary(VMContext, Elts); + return MDNode::getTemporary(VMContext, Elts).release(); }); } diff --git a/lib/IR/MDBuilder.cpp b/lib/IR/MDBuilder.cpp index c7fcf7a2c34..d6d03b48d0f 100644 --- a/lib/IR/MDBuilder.cpp +++ b/lib/IR/MDBuilder.cpp @@ -68,9 +68,9 @@ MDNode *MDBuilder::createRange(const APInt &Lo, const APInt &Hi) { MDNode *MDBuilder::createAnonymousAARoot(StringRef Name, MDNode *Extra) { // To ensure uniqueness the root node is self-referential. - MDNode *Dummy = MDNode::getTemporary(Context, None); + auto Dummy = MDNode::getTemporary(Context, None); - SmallVector Args(1, Dummy); + SmallVector Args(1, Dummy.get()); if (Extra) Args.push_back(Extra); if (!Name.empty()) @@ -82,7 +82,7 @@ MDNode *MDBuilder::createAnonymousAARoot(StringRef Name, MDNode *Extra) { // !1 = metadata !{metadata !0} <- root // Replace the dummy operand with the root node itself and delete the dummy. Root->replaceOperandWith(0, Root); - MDNode::deleteTemporary(Dummy); + // We now have // !1 = metadata !{metadata !1} <- self-referential root return Root; diff --git a/lib/Transforms/Utils/InlineFunction.cpp b/lib/Transforms/Utils/InlineFunction.cpp index 3cebd232984..f14afdb78cb 100644 --- a/lib/Transforms/Utils/InlineFunction.cpp +++ b/lib/Transforms/Utils/InlineFunction.cpp @@ -319,13 +319,12 @@ static void CloneAliasScopeMetadata(CallSite CS, ValueToValueMapTy &VMap) { // Now we have a complete set of all metadata in the chains used to specify // the noalias scopes and the lists of those scopes. - SmallVector DummyNodes; + SmallVector DummyNodes; DenseMap MDMap; for (SetVector::iterator I = MD.begin(), IE = MD.end(); I != IE; ++I) { - MDTuple *Dummy = MDTuple::getTemporary(CalledFunc->getContext(), None); - DummyNodes.push_back(Dummy); - MDMap[*I].reset(Dummy); + DummyNodes.push_back(MDTuple::getTemporary(CalledFunc->getContext(), None)); + MDMap[*I].reset(DummyNodes.back().get()); } // Create new metadata nodes to replace the dummy nodes, replacing old @@ -389,10 +388,6 @@ static void CloneAliasScopeMetadata(CallSite CS, ValueToValueMapTy &VMap) { NI->setMetadata(LLVMContext::MD_noalias, M); } } - - // Now that everything has been replaced, delete the dummy nodes. - for (unsigned i = 0, ie = DummyNodes.size(); i != ie; ++i) - MDNode::deleteTemporary(DummyNodes[i]); } /// AddAliasScopeMetadata - If the inlined function has noalias arguments, then diff --git a/lib/Transforms/Utils/ValueMapper.cpp b/lib/Transforms/Utils/ValueMapper.cpp index b195c6349b7..528bf373fff 100644 --- a/lib/Transforms/Utils/ValueMapper.cpp +++ b/lib/Transforms/Utils/ValueMapper.cpp @@ -249,12 +249,11 @@ static Metadata *mapDistinctNode(const UniquableMDNode *Node, // In general we need a dummy node, since whether the operands are null can // affect the size of the node. - MDTuple *Dummy = MDTuple::getTemporary(Node->getContext(), None); - mapToMetadata(VM, Node, Dummy); + auto Dummy = MDTuple::getTemporary(Node->getContext(), None); + mapToMetadata(VM, Node, Dummy.get()); Metadata *NewMD = cloneMDNode(Node, VM, Flags, TypeMapper, Materializer, /* IsDistinct */ true); Dummy->replaceAllUsesWith(NewMD); - MDNode::deleteTemporary(Dummy); return mapToMetadata(VM, Node, NewMD); } @@ -285,14 +284,13 @@ static Metadata *mapUniquedNode(const UniquableMDNode *Node, assert(Node->isUniqued() && "Expected uniqued node"); // Create a dummy node in case we have a metadata cycle. - MDTuple *Dummy = MDTuple::getTemporary(Node->getContext(), None); - mapToMetadata(VM, Node, Dummy); + auto Dummy = MDTuple::getTemporary(Node->getContext(), None); + mapToMetadata(VM, Node, Dummy.get()); // Check all operands to see if any need to be remapped. if (!shouldRemapUniquedNode(Node, VM, Flags, TypeMapper, Materializer)) { // Use an identity mapping. mapToSelf(VM, Node); - MDNode::deleteTemporary(Dummy); return const_cast(static_cast(Node)); } @@ -300,7 +298,6 @@ static Metadata *mapUniquedNode(const UniquableMDNode *Node, Metadata *NewMD = cloneMDNode(Node, VM, Flags, TypeMapper, Materializer, /* IsDistinct */ false); Dummy->replaceAllUsesWith(NewMD); - MDNode::deleteTemporary(Dummy); return mapToMetadata(VM, Node, NewMD); } diff --git a/unittests/IR/MetadataTest.cpp b/unittests/IR/MetadataTest.cpp index 498728e3cac..de2ff8f81d9 100644 --- a/unittests/IR/MetadataTest.cpp +++ b/unittests/IR/MetadataTest.cpp @@ -171,11 +171,10 @@ TEST_F(MDNodeTest, SelfReference) { // !0 = !{!0} // !1 = !{!0} { - MDNode *Temp = MDNode::getTemporary(Context, None); - Metadata *Args[] = {Temp}; + auto Temp = MDNode::getTemporary(Context, None); + Metadata *Args[] = {Temp.get()}; MDNode *Self = MDNode::get(Context, Args); Self->replaceOperandWith(0, Self); - MDNode::deleteTemporary(Temp); ASSERT_EQ(Self, Self->getOperand(0)); // Self-references should be distinct, so MDNode::get() should grab a @@ -190,11 +189,10 @@ TEST_F(MDNodeTest, SelfReference) { // !0 = !{!0, !{}} // !1 = !{!0, !{}} { - MDNode *Temp = MDNode::getTemporary(Context, None); - Metadata *Args[] = {Temp, MDNode::get(Context, None)}; + auto Temp = MDNode::getTemporary(Context, None); + Metadata *Args[] = {Temp.get(), MDNode::get(Context, None)}; MDNode *Self = MDNode::get(Context, Args); Self->replaceOperandWith(0, Self); - MDNode::deleteTemporary(Temp); ASSERT_EQ(Self, Self->getOperand(0)); // Self-references should be distinct, so MDNode::get() should grab a @@ -310,48 +308,44 @@ TEST_F(MDNodeTest, getDistinct) { TEST_F(MDNodeTest, isUniqued) { MDNode *U = MDTuple::get(Context, None); MDNode *D = MDTuple::getDistinct(Context, None); - MDNode *T = MDTuple::getTemporary(Context, None); + auto T = MDTuple::getTemporary(Context, None); EXPECT_TRUE(U->isUniqued()); EXPECT_FALSE(D->isUniqued()); EXPECT_FALSE(T->isUniqued()); - MDNode::deleteTemporary(T); } TEST_F(MDNodeTest, isDistinct) { MDNode *U = MDTuple::get(Context, None); MDNode *D = MDTuple::getDistinct(Context, None); - MDNode *T = MDTuple::getTemporary(Context, None); + auto T = MDTuple::getTemporary(Context, None); EXPECT_FALSE(U->isDistinct()); EXPECT_TRUE(D->isDistinct()); EXPECT_FALSE(T->isDistinct()); - MDNode::deleteTemporary(T); } TEST_F(MDNodeTest, isTemporary) { MDNode *U = MDTuple::get(Context, None); MDNode *D = MDTuple::getDistinct(Context, None); - MDNode *T = MDTuple::getTemporary(Context, None); + auto T = MDTuple::getTemporary(Context, None); EXPECT_FALSE(U->isTemporary()); EXPECT_FALSE(D->isTemporary()); EXPECT_TRUE(T->isTemporary()); - MDNode::deleteTemporary(T); } TEST_F(MDNodeTest, getDistinctWithUnresolvedOperands) { // temporary !{} - MDTuple *Temp = MDTuple::getTemporary(Context, None); + auto Temp = MDTuple::getTemporary(Context, None); ASSERT_FALSE(Temp->isResolved()); // distinct !{temporary !{}} - Metadata *Ops[] = {Temp}; + Metadata *Ops[] = {Temp.get()}; MDNode *Distinct = MDNode::getDistinct(Context, Ops); EXPECT_TRUE(Distinct->isResolved()); - EXPECT_EQ(Temp, Distinct->getOperand(0)); + EXPECT_EQ(Temp.get(), Distinct->getOperand(0)); // temporary !{} => !{} MDNode *Empty = MDNode::get(Context, None); Temp->replaceAllUsesWith(Empty); - MDNode::deleteTemporary(Temp); EXPECT_EQ(Empty, Distinct->getOperand(0)); } @@ -360,19 +354,18 @@ TEST_F(MDNodeTest, handleChangedOperandRecursion) { MDNode *N0 = MDNode::get(Context, None); // !1 = !{!3, null} - MDTuple *Temp3 = MDTuple::getTemporary(Context, None); - Metadata *Ops1[] = {Temp3, nullptr}; + auto Temp3 = MDTuple::getTemporary(Context, None); + Metadata *Ops1[] = {Temp3.get(), nullptr}; MDNode *N1 = MDNode::get(Context, Ops1); // !2 = !{!3, !0} - Metadata *Ops2[] = {Temp3, N0}; + Metadata *Ops2[] = {Temp3.get(), N0}; MDNode *N2 = MDNode::get(Context, Ops2); // !3 = !{!2} Metadata *Ops3[] = {N2}; MDNode *N3 = MDNode::get(Context, Ops3); Temp3->replaceAllUsesWith(N3); - MDNode::deleteTemporary(Temp3); // !4 = !{!1} Metadata *Ops4[] = {N1}; @@ -425,8 +418,8 @@ TEST_F(MDNodeTest, replaceResolvedOperand) { // a global value that gets RAUW'ed. // // Use a temporary node to keep N from being resolved. - MDTuple *Temp = MDTuple::getTemporary(Context, None); - Metadata *Ops[] = {nullptr, Temp}; + auto Temp = MDTuple::getTemporary(Context, None); + Metadata *Ops[] = {nullptr, Temp.get()}; MDNode *Empty = MDTuple::get(Context, ArrayRef()); MDNode *N = MDTuple::get(Context, Ops); @@ -438,12 +431,11 @@ TEST_F(MDNodeTest, replaceResolvedOperand) { EXPECT_EQ(Empty, N->getOperand(0)); // Check code for adding another unresolved operand. - N->replaceOperandWith(0, Temp); - EXPECT_EQ(Temp, N->getOperand(0)); + N->replaceOperandWith(0, Temp.get()); + EXPECT_EQ(Temp.get(), N->getOperand(0)); // Remove the references to Temp; required for teardown. Temp->replaceAllUsesWith(nullptr); - MDNode::deleteTemporary(Temp); } typedef MetadataTest MDLocationTest; @@ -485,10 +477,9 @@ TEST_F(MDLocationTest, getDistinct) { TEST_F(MDLocationTest, getTemporary) { MDNode *N = MDNode::get(Context, None); - MDLocation *L = MDLocation::getTemporary(Context, 2, 7, N); + auto L = MDLocation::getTemporary(Context, 2, 7, N); EXPECT_TRUE(L->isTemporary()); EXPECT_FALSE(L->isResolved()); - MDNode::deleteTemporary(L); } typedef MetadataTest MetadataAsValueTest; @@ -557,11 +548,11 @@ TEST_F(ValueAsMetadataTest, CollidingDoubleUpdates) { ConstantInt::get(getGlobalContext(), APInt(8, 0))); // Create a temporary to prevent nodes from resolving. - MDTuple *Temp = MDTuple::getTemporary(Context, None); + auto Temp = MDTuple::getTemporary(Context, None); // When the first operand of N1 gets reset to nullptr, it'll collide with N2. - Metadata *Ops1[] = {CI, CI, Temp}; - Metadata *Ops2[] = {nullptr, CI, Temp}; + Metadata *Ops1[] = {CI, CI, Temp.get()}; + Metadata *Ops2[] = {nullptr, CI, Temp.get()}; auto *N1 = MDTuple::get(Context, Ops1); auto *N2 = MDTuple::get(Context, Ops2); @@ -573,11 +564,10 @@ TEST_F(ValueAsMetadataTest, CollidingDoubleUpdates) { ValueAsMetadata::handleDeletion(CI->getValue()); EXPECT_EQ(nullptr, N2->getOperand(0)); EXPECT_EQ(nullptr, N2->getOperand(1)); - EXPECT_EQ(Temp, N2->getOperand(2)); + EXPECT_EQ(Temp.get(), N2->getOperand(2)); // Clean up Temp for teardown. Temp->replaceAllUsesWith(nullptr); - MDNode::deleteTemporary(Temp); } typedef MetadataTest TrackingMDRefTest; -- 2.34.1