IR: Remove MDNodeFwdDecl

Remove `MDNodeFwdDecl` (as promised in r226481).  Aside from API
changes, there's no real functionality change here.
`MDNode::getTemporary()` now forwards to `MDTuple::getTemporary()`,
which returns a tuple with `isTemporary()` equal to true.

The main point is that we can now add temporaries of other `MDNode`
subclasses, needed for PR22235 (I introduced `MDNodeFwdDecl` in the
first place because I didn't recognize this need, and thought they were
only needed to handle forward references).

A few things left out of (or highlighted by) this commit:

  - I've had to remove the (few) uses of `std::unique_ptr<>` to deal
    with temporaries, since the destructor is no longer public.
    `getTemporary()` should probably return the equivalent of
    `std::unique_ptr<T, MDNode::deleteTemporary>`.
  - `MDLocation::getTemporary()` doesn't exist yet (worse, it actually
    does exist, but does the wrong thing: `MDNode::getTemporary()` is
    inherited and returns an `MDTuple`).
  - `MDNode` now only has one subclass, `UniquableMDNode`, and the
    distinction between them is actually somewhat confusing.

I'll fix those up next.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@226501 91177308-0d34-0410-b5e6-96231b3b80d8

bindings/go/llvm/IRBindings.cpp

@@ -86,7 +86,8 @@ void LLVMSetMetadata2(LLVMValueRef Inst, unsigned KindID, LLVMMetadataRef MD) {
 }
 
 void LLVMMetadataReplaceAllUsesWith(LLVMMetadataRef MD, LLVMMetadataRef New) {
-  auto *Node = unwrap<MDNodeFwdDecl>(MD);
+  auto *Node = unwrap<MDTuple>(MD);
+  assert(Node->isTemporary() && "Expected temporary node");
   Node->replaceAllUsesWith(unwrap<MDNode>(New));
   MDNode::deleteTemporary(Node);
 }

include/llvm/IR/DIBuilder.h

@@ -85,7 +85,7 @@ namespace llvm {
 
     /// \brief Create a temporary.
     ///
-    /// Create an \a MDNodeFwdDecl and track it in \a UnresolvedNodes.
+    /// Create an \a temporary node and track it in \a UnresolvedNodes.
     void trackIfUnresolved(MDNode *N);
 
   public:

include/llvm/IR/Metadata.def

@@ -47,7 +47,6 @@ HANDLE_METADATA_BRANCH(ValueAsMetadata)
 HANDLE_METADATA_LEAF(ConstantAsMetadata)
 HANDLE_METADATA_LEAF(LocalAsMetadata)
 HANDLE_METADATA_BRANCH(MDNode)
-HANDLE_METADATA_LEAF(MDNodeFwdDecl)
 HANDLE_UNIQUABLE_BRANCH(UniquableMDNode)
 HANDLE_UNIQUABLE_LEAF(MDTuple)
 HANDLE_UNIQUABLE_LEAF(MDLocation)

include/llvm/IR/Metadata.h

@@ -61,7 +61,6 @@ public:
   enum MetadataKind {
     MDTupleKind,
     MDLocationKind,
-    MDNodeFwdDeclKind,
     ConstantAsMetadataKind,
     LocalAsMetadataKind,
     MDStringKind
@@ -698,14 +697,8 @@ public:
                                      ArrayRef<Metadata *> MDs);
   static inline MDTuple *getDistinct(LLVMContext &Context,
                                      ArrayRef<Metadata *> MDs);
-
+  static inline MDTuple *getTemporary(LLVMContext &Context,
-  /// \brief Return a temporary MDNode
+                                      ArrayRef<Metadata *> MDs);
-  ///
-  /// 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 MDNodeFwdDecl *getTemporary(LLVMContext &Context,
-                                     ArrayRef<Metadata *> MDs);
 
   /// \brief Deallocate a node created by getTemporary.
   ///
@@ -772,8 +765,7 @@ public:
   /// \brief Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == MDTupleKind ||
-           MD->getMetadataID() == MDLocationKind ||
+           MD->getMetadataID() == MDLocationKind;
-           MD->getMetadataID() == MDNodeFwdDeclKind;
   }
 
   /// \brief Check whether MDNode is a vtable access.
@@ -794,16 +786,15 @@ public:
 /// for implementing sub-types of \a MDNode that can be uniqued like
 /// constants.
 ///
-/// There is limited support for RAUW at construction time.  At
+/// There is limited support for RAUW at construction time.  At construction
-/// construction time, if any operands are an instance of \a
+/// time, if any operand is a temporary node (or an unresolved uniqued node,
-/// MDNodeFwdDecl (or another unresolved \a UniquableMDNode, which
+/// which indicates a transitive temporary operand), the node itself will be
-/// indicates an \a MDNodeFwdDecl in its path), the node itself will be
+/// unresolved.  As soon as all operands become resolved, it will drop RAUW
-/// unresolved.  As soon as all operands become resolved, it will drop
+/// support permanently.
-/// RAUW support permanently.
 ///
 /// If an unresolved node is part of a cycle, \a resolveCycles() needs
-/// to be called on some member of the cycle when each \a MDNodeFwdDecl
+/// to be called on some member of the cycle once all temporary nodes have been
-/// has been removed.
+/// replaced.
 class UniquableMDNode : public MDNode {
   friend class ReplaceableMetadataImpl;
   friend class MDNode;
@@ -834,7 +825,7 @@ public:
   /// Once all forward declarations have been resolved, force cycles to be
   /// resolved.
   ///
-  /// \pre No operands (or operands' operands, etc.) are \a MDNodeFwdDecl.
+  /// \pre No operands (or operands' operands, etc.) have \a isTemporary().
   void resolveCycles();
 
 private:
@@ -888,6 +879,15 @@ public:
     return getImpl(Context, MDs, Distinct);
   }
 
+  /// \brief Return a temporary node.
+  ///
+  /// 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<Metadata *> MDs) {
+    return getImpl(Context, MDs, Temporary);
+  }
+
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == MDTupleKind;
   }
@@ -906,6 +906,9 @@ MDTuple *MDNode::getIfExists(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
 MDTuple *MDNode::getDistinct(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
   return MDTuple::getDistinct(Context, MDs);
 }
+MDTuple *MDNode::getTemporary(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
+  return MDTuple::getTemporary(Context, MDs);
+}
 
 /// \brief Debug location.
 ///
@@ -961,32 +964,6 @@ private:
   void eraseFromStoreImpl();
 };
 
-/// \brief Forward declaration of metadata.
-///
-/// Forward declaration of metadata, in the form of a basic tuple.  Unlike \a
-/// MDTuple, this class has full support for RAUW, is not owned, is not
-/// uniqued, and is suitable for forward references.
-class MDNodeFwdDecl : public MDNode {
-  friend class Metadata;
-
-  MDNodeFwdDecl(LLVMContext &C, ArrayRef<Metadata *> Vals)
-      : MDNode(C, MDNodeFwdDeclKind, Temporary, Vals) {}
-
-public:
-  ~MDNodeFwdDecl() { dropAllReferences(); }
-
-  // MSVC doesn't see the alternative: "using MDNode::operator delete".
-  void operator delete(void *Mem) { MDNode::operator delete(Mem); }
-
-  static MDNodeFwdDecl *get(LLVMContext &Context, ArrayRef<Metadata *> MDs) {
-    return new (MDs.size()) MDNodeFwdDecl(Context, MDs);
-  }
-
-  static bool classof(const Metadata *MD) {
-    return MD->getMetadataID() == MDNodeFwdDeclKind;
-  }
-};
-
 //===----------------------------------------------------------------------===//
 /// \brief A tuple of MDNodes.
 ///

lib/AsmParser/LLParser.cpp

@@ -531,7 +531,7 @@ bool LLParser::ParseMDNodeID(MDNode *&Result) {
   }
 
   // Otherwise, create MDNode forward reference.
-  MDNodeFwdDecl *FwdNode = MDNodeFwdDecl::get(Context, None);
+  MDTuple *FwdNode = MDTuple::getTemporary(Context, None);
   ForwardRefMDNodes[MID] = std::make_pair(FwdNode, Lex.getLoc());
 
   if (NumberedMetadata.size() <= MID)
@@ -597,9 +597,9 @@ bool LLParser::ParseStandaloneMetadata() {
   // See if this was forward referenced, if so, handle it.
   auto FI = ForwardRefMDNodes.find(MetadataID);
   if (FI != ForwardRefMDNodes.end()) {
-    MDNodeFwdDecl *Temp = FI->second.first;
+    MDTuple *Temp = FI->second.first;
     Temp->replaceAllUsesWith(Init);
-    delete Temp;
+    MDNode::deleteTemporary(Temp);
     ForwardRefMDNodes.erase(FI);
 
     assert(NumberedMetadata[MetadataID] == Init && "Tracking VH didn't work");

lib/AsmParser/LLParser.h

@@ -136,7 +136,7 @@ namespace llvm {
     std::vector<std::pair<Type*, LocTy> > NumberedTypes;
 
     std::vector<TrackingMDNodeRef> NumberedMetadata;
-    std::map<unsigned, std::pair<MDNodeFwdDecl *, LocTy>> ForwardRefMDNodes;
+    std::map<unsigned, std::pair<MDTuple *, LocTy>> ForwardRefMDNodes;
 
     // Global Value reference information.
     std::map<std::string, std::pair<GlobalValue*, LocTy> > ForwardRefVals;

lib/Bitcode/Reader/BitcodeReader.cpp

@@ -541,7 +541,7 @@ void BitcodeReaderMDValueList::AssignValue(Metadata *MD, unsigned Idx) {
   }
 
   // If there was a forward reference to this value, replace it.
-  MDNodeFwdDecl *PrevMD = cast<MDNodeFwdDecl>(OldMD.get());
+  MDTuple *PrevMD = cast<MDTuple>(OldMD.get());
   PrevMD->replaceAllUsesWith(MD);
   MDNode::deleteTemporary(PrevMD);
   --NumFwdRefs;
@@ -573,9 +573,12 @@ void BitcodeReaderMDValueList::tryToResolveCycles() {
 
   // Resolve any cycles.
   for (auto &MD : MDValuePtrs) {
-    assert(!(MD && isa<MDNodeFwdDecl>(MD)) && "Unexpected forward reference");
+    auto *N = dyn_cast_or_null<UniquableMDNode>(MD);
-    if (auto *N = dyn_cast_or_null<UniquableMDNode>(MD))
+    if (!N)
-      N->resolveCycles();
+      continue;
+
+    assert(!N->isTemporary() && "Unexpected forward reference");
+    N->resolveCycles();
   }
 }
 

lib/IR/DebugInfo.cpp

@@ -337,7 +337,8 @@ void DIDescriptor::replaceAllUsesWith(LLVMContext &VMContext, DIDescriptor D) {
     DN = MDNode::get(VMContext, Ops);
   }
 
-  auto *Node = cast<MDNodeFwdDecl>(const_cast<MDNode *>(DbgNode));
+  assert(DbgNode->isTemporary() && "Expected temporary node");
+  auto *Node = const_cast<MDNode *>(DbgNode);
   Node->replaceAllUsesWith(const_cast<MDNode *>(DN));
   MDNode::deleteTemporary(Node);
   DbgNode = DN;
@@ -346,7 +347,8 @@ void DIDescriptor::replaceAllUsesWith(LLVMContext &VMContext, DIDescriptor D) {
 void DIDescriptor::replaceAllUsesWith(MDNode *D) {
   assert(DbgNode && "Trying to replace an unverified type!");
   assert(DbgNode != D && "This replacement should always happen");
-  auto *Node = cast<MDNodeFwdDecl>(const_cast<MDNode *>(DbgNode));
+  assert(DbgNode->isTemporary() && "Expected temporary node");
+  auto *Node = const_cast<MDNode *>(DbgNode);
   Node->replaceAllUsesWith(D);
   MDNode::deleteTemporary(Node);
 }

lib/IR/Metadata.cpp

@@ -155,7 +155,8 @@ void ReplaceableMetadataImpl::moveRef(void *Ref, void *New,
 }
 
 void ReplaceableMetadataImpl::replaceAllUsesWith(Metadata *MD) {
-  assert(!(MD && isa<MDNodeFwdDecl>(MD)) && "Expected non-temp node");
+  assert(!(MD && isa<MDNode>(MD) && cast<MDNode>(MD)->isTemporary()) &&
+         "Expected non-temp node");
 
   if (UseMap.empty())
     return;
@@ -471,13 +472,14 @@ void UniquableMDNode::resolveCycles() {
 
   // Resolve all operands.
   for (const auto &Op : operands()) {
-    if (!Op)
+    auto *N = dyn_cast_or_null<UniquableMDNode>(Op);
+    if (!N)
       continue;
-    assert(!isa<MDNodeFwdDecl>(Op) &&
+
+    assert(!N->isTemporary() &&
            "Expected all forward declarations to be resolved");
-    if (auto *N = dyn_cast<UniquableMDNode>(Op))
+    if (!N->isResolved())
-      if (!N->isResolved())
+      N->resolveCycles();
-        N->resolveCycles();
   }
 }
 
@@ -621,7 +623,7 @@ T *UniquableMDNode::storeImpl(T *N, StorageType Storage, StoreT &Store) {
     N->storeDistinctInContext();
     break;
   case Temporary:
-    llvm_unreachable("Unexpected temporary node");
+    break;
   }
   return N;
 }
@@ -723,13 +725,11 @@ void MDLocation::eraseFromStoreImpl() {
   getContext().pImpl->MDLocations.erase(this);
 }
 
-MDNodeFwdDecl *MDNode::getTemporary(LLVMContext &Context,
+void MDNode::deleteTemporary(MDNode *N) {
-                                    ArrayRef<Metadata *> MDs) {
+  assert(N->isTemporary() && "Expected temporary node");
-  return MDNodeFwdDecl::get(Context, MDs);
+  cast<UniquableMDNode>(N)->deleteAsSubclass();
 }
 
-void MDNode::deleteTemporary(MDNode *N) { delete cast<MDNodeFwdDecl>(N); }
-
 void UniquableMDNode::storeDistinctInContext() {
   assert(isResolved() && "Expected resolved nodes");
   Storage = Distinct;

lib/IR/Verifier.cpp

@@ -617,7 +617,7 @@ void Verifier::visitMDNode(MDNode &MD) {
   }
 
   // Check these last, so we diagnose problems in operands first.
-  Assert1(!isa<MDNodeFwdDecl>(MD), "Expected no forward declarations!", &MD);
+  Assert1(!MD.isTemporary(), "Expected no forward declarations!", &MD);
   Assert1(MD.isResolved(), "All nodes should be resolved!", &MD);
 }
 

lib/Transforms/Utils/InlineFunction.cpp

@@ -319,11 +319,11 @@ 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<MDNode *, 16> DummyNodes;
+  SmallVector<MDTuple *, 16> DummyNodes;
   DenseMap<const MDNode *, TrackingMDNodeRef> MDMap;
   for (SetVector<const MDNode *>::iterator I = MD.begin(), IE = MD.end();
        I != IE; ++I) {
-    MDNode *Dummy = MDNode::getTemporary(CalledFunc->getContext(), None);
+    MDTuple *Dummy = MDTuple::getTemporary(CalledFunc->getContext(), None);
     DummyNodes.push_back(Dummy);
     MDMap[*I].reset(Dummy);
   }
@@ -343,7 +343,8 @@ static void CloneAliasScopeMetadata(CallSite CS, ValueToValueMapTy &VMap) {
     }
 
     MDNode *NewM = MDNode::get(CalledFunc->getContext(), NewOps);
-    MDNodeFwdDecl *TempM = cast<MDNodeFwdDecl>(MDMap[*I]);
+    MDTuple *TempM = cast<MDTuple>(MDMap[*I]);
+    assert(TempM->isTemporary() && "Expected temporary node");
 
     TempM->replaceAllUsesWith(NewM);
   }

lib/Transforms/Utils/ValueMapper.cpp

@@ -249,12 +249,12 @@ 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.
-  std::unique_ptr<MDNodeFwdDecl> Dummy(
+  MDTuple *Dummy = MDTuple::getTemporary(Node->getContext(), None);
-      MDNode::getTemporary(Node->getContext(), None));
+  mapToMetadata(VM, Node, Dummy);
-  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,7 +285,7 @@ static Metadata *mapUniquedNode(const UniquableMDNode *Node,
   assert(Node->isUniqued() && "Expected uniqued node");
 
   // Create a dummy node in case we have a metadata cycle.
-  MDNodeFwdDecl *Dummy = MDNode::getTemporary(Node->getContext(), None);
+  MDTuple *Dummy = MDTuple::getTemporary(Node->getContext(), None);
   mapToMetadata(VM, Node, Dummy);
 
   // Check all operands to see if any need to be remapped.

unittests/IR/MetadataTest.cpp

@@ -167,10 +167,6 @@ TEST_F(MDNodeTest, Delete) {
   delete I;
 }
 
-TEST_F(MDNodeTest, DeleteMDNodeFwdDecl) {
-  delete MDNode::getTemporary(Context, None);
-}
-
 TEST_F(MDNodeTest, SelfReference) {
   // !0 = !{!0}
   // !1 = !{!0}
@@ -343,7 +339,7 @@ TEST_F(MDNodeTest, isTemporary) {
 
 TEST_F(MDNodeTest, getDistinctWithUnresolvedOperands) {
   // temporary !{}
-  MDNodeFwdDecl *Temp = MDNode::getTemporary(Context, None);
+  MDTuple *Temp = MDTuple::getTemporary(Context, None);
   ASSERT_FALSE(Temp->isResolved());
 
   // distinct !{temporary !{}}
@@ -364,18 +360,19 @@ TEST_F(MDNodeTest, handleChangedOperandRecursion) {
   MDNode *N0 = MDNode::get(Context, None);
 
   // !1 = !{!3, null}
-  std::unique_ptr<MDNodeFwdDecl> Temp3(MDNode::getTemporary(Context, None));
+  MDTuple *Temp3 = MDTuple::getTemporary(Context, None);
-  Metadata *Ops1[] = {Temp3.get(), nullptr};
+  Metadata *Ops1[] = {Temp3, nullptr};
   MDNode *N1 = MDNode::get(Context, Ops1);
 
   // !2 = !{!3, !0}
-  Metadata *Ops2[] = {Temp3.get(), N0};
+  Metadata *Ops2[] = {Temp3, 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};
@@ -428,8 +425,8 @@ TEST_F(MDNodeTest, replaceResolvedOperand) {
   // a global value that gets RAUW'ed.
   //
   // Use a temporary node to keep N from being resolved.
-  std::unique_ptr<MDNodeFwdDecl> Temp(MDNodeFwdDecl::get(Context, None));
+  MDTuple *Temp = MDTuple::getTemporary(Context, None);
-  Metadata *Ops[] = {nullptr, Temp.get()};
+  Metadata *Ops[] = {nullptr, Temp};
 
   MDNode *Empty = MDTuple::get(Context, ArrayRef<Metadata *>());
   MDNode *N = MDTuple::get(Context, Ops);
@@ -441,11 +438,12 @@ TEST_F(MDNodeTest, replaceResolvedOperand) {
   EXPECT_EQ(Empty, N->getOperand(0));
 
   // Check code for adding another unresolved operand.
-  N->replaceOperandWith(0, Temp.get());
+  N->replaceOperandWith(0, Temp);
-  EXPECT_EQ(Temp.get(), N->getOperand(0));
+  EXPECT_EQ(Temp, N->getOperand(0));
 
   // Remove the references to Temp; required for teardown.
   Temp->replaceAllUsesWith(nullptr);
+  MDNode::deleteTemporary(Temp);
 }
 
 typedef MetadataTest MDLocationTest;
@@ -551,11 +549,11 @@ TEST_F(ValueAsMetadataTest, CollidingDoubleUpdates) {
       ConstantInt::get(getGlobalContext(), APInt(8, 0)));
 
   // Create a temporary to prevent nodes from resolving.
-  std::unique_ptr<MDNodeFwdDecl> Temp(MDNode::getTemporary(Context, None));
+  MDTuple *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.get()};
+  Metadata *Ops1[] = {CI, CI, Temp};
-  Metadata *Ops2[] = {nullptr, CI, Temp.get()};
+  Metadata *Ops2[] = {nullptr, CI, Temp};
 
   auto *N1 = MDTuple::get(Context, Ops1);
   auto *N2 = MDTuple::get(Context, Ops2);
@@ -567,10 +565,11 @@ TEST_F(ValueAsMetadataTest, CollidingDoubleUpdates) {
   ValueAsMetadata::handleDeletion(CI->getValue());
   EXPECT_EQ(nullptr, N2->getOperand(0));
   EXPECT_EQ(nullptr, N2->getOperand(1));
-  EXPECT_EQ(Temp.get(), N2->getOperand(2));
+  EXPECT_EQ(Temp, N2->getOperand(2));
 
   // Clean up Temp for teardown.
   Temp->replaceAllUsesWith(nullptr);
+  MDNode::deleteTemporary(Temp);
 }
 
 typedef MetadataTest TrackingMDRefTest;