[LLVM-C] Add Bindings For Named Metadata

[CodaFi]

Summary

Add a new type for named metadata nodes. Use this to implement iterators and accessors for NamedMDNodes and extend the echo test to use them to copy module-level debug information.

[CodaFi]

@aprantl Let me know if this is the right way to address that FIXME. There didn't seem to be any consequences of the small change I've got here which could be very good or very bad from my naive perspective.

[whitequark]

C API changes LGTM.

[aprantl]

Adding Duncan to the review list who (presumably) added the TODO and may have some thoughts on this.

[dexonsmith]

In D47179#1107993, @aprantl wrote:

Adding Duncan to the review list who (presumably) added the TODO and may have some thoughts on this.

Making NamedMDNode inherit from Metadata will allow MDNode to reference NamedMDNode.

• What's the use case for this?
• What complexity does it add to the IR? (Especially, during serialization and LTO...)

Perhaps this is worth doing, but we need a compelling reason. In retrospect, I'm not sure I had sufficient motiviation when I added the TODO.

[dexonsmith]

In D47179#1108484, @dexonsmith wrote:

In D47179#1107993, @aprantl wrote:

Adding Duncan to the review list who (presumably) added the TODO and may have some thoughts on this.

Making NamedMDNode inherit from Metadata will allow MDNode to reference NamedMDNode.

• What's the use case for this?
• What complexity does it add to the IR? (Especially, during serialization and LTO...)

Perhaps this is worth doing, but we need a compelling reason. In retrospect, I'm not sure I had sufficient motiviation when I added the TODO.

If the goal is just to expose them to the C API, you can expose them as is by adding LLVMNamedMetadataRef, etc.

[CodaFi]

Spoke with @aprantl in person about this, and we came to the conclusion that named metadata nodes are different enough that the original TODO doesn't make any sense. For one, it would mean that you could use named metadata nodes to create more kinds of cycles. It would also mean the verifier would have to be extended to reject any usage of named metadata nodes as operand metadata.

Suffice to say, it's both simpler and more correct to make a new type in the C API and wrap to that.

[aprantl]

Could you perhaps replace the TODO in the code with a summary of what you just said?

[Wallbraker]

The C-API changes looks good to me and the logic makes sense. Just one nit.

include/llvm-c/Core.h

898 ↗

(On Diff #162569)

Should this be disposed with LLVMDisposeMessage? Please update comment to reflect this.

[CodaFi]

include/llvm-c/Core.h

898 ↗

(On Diff #162569)

No. In general, Get-named things return objects managed by somebody else. Copy-named things (or, unfortunately, functions documented to the contrary) return user-owned objects. In this case, the module owns the named metadata which owns its name.

[Wallbraker]

include/llvm-c/Core.h

898 ↗

(On Diff #162569)

Ah okay, so it's not consistent over the API? Even more reason to put it in the doc-comment of this function.

[CodaFi]

include/llvm-c/Core.h

898 ↗

(On Diff #162569)

The exceptional cases are what deserve doc comments, and they are documented. Otherwise I would have to re-annotate the entire C API which is out of scope for this patch and would be noise.

It would be good to go through and audit these things, though. The C API is quite old and could use some standardization here.

[CodaFi]

@whitequark Could I get another once-over on the C API here?

[whitequark]

LGTM

llvm/trunk/include/llvm-c/Core.h

 LLVMContextRef LLVMGetModuleContext(LLVMModuleRef M);
 
 /**
  * Obtain a Type from a module by its registered name.
  */
 LLVMTypeRef LLVMGetTypeByName(LLVMModuleRef M, const char *Name);
 
 /**
+ * Obtain an iterator to the first NamedMDNode in a Module.
+ *
+ * @see llvm::Module::named_metadata_begin()
+ */
+LLVMNamedMDNodeRef LLVMGetFirstNamedMetadata(LLVMModuleRef M);
+
+/**
+ * Obtain an iterator to the last NamedMDNode in a Module.
+ *
+ * @see llvm::Module::named_metadata_end()
+ */
+LLVMNamedMDNodeRef LLVMGetLastNamedMetadata(LLVMModuleRef M);
+
+/**
+ * Advance a NamedMDNode iterator to the next NamedMDNode.
+ *
+ * Returns NULL if the iterator was already at the end and there are no more
+ * named metadata nodes.
+ */
+LLVMNamedMDNodeRef LLVMGetNextNamedMetadata(LLVMNamedMDNodeRef NamedMDNode);
+
+/**
+ * Decrement a NamedMDNode iterator to the previous NamedMDNode.
+ *
+ * Returns NULL if the iterator was already at the beginning and there are
+ * no previous named metadata nodes.
+ */
+LLVMNamedMDNodeRef LLVMGetPreviousNamedMetadata(LLVMNamedMDNodeRef NamedMDNode);
+
+/**
+ * Retrieve a NamedMDNode with the given name, returning NULL if no such
+ * node exists.
+ *
+ * @see llvm::Module::getNamedMetadata()
+ */
+LLVMNamedMDNodeRef LLVMGetNamedMetadata(LLVMModuleRef M,
+                                        const char *Name, size_t NameLen);
+
+/**
+ * Retrieve a NamedMDNode with the given name, creating a new node if no such
+ * node exists.
+ *
+ * @see llvm::Module::getOrInsertNamedMetadata()
+ */
+LLVMNamedMDNodeRef LLVMGetOrInsertNamedMetadata(LLVMModuleRef M,
+                                                const char *Name,
+                                                size_t NameLen);
+
+/**
+ * Retrieve the name of a NamedMDNode.
+ *
+ * @see llvm::NamedMDNode::getName()
+ */
+const char *LLVMGetNamedMetadataName(LLVMNamedMDNodeRef NamedMD,
+                                     size_t *NameLen);
+
+/**
  * Obtain the number of operands for named metadata in a module.
  *
  * @see llvm::Module::getNamedMetadata()
  */
 unsigned LLVMGetNamedMetadataNumOperands(LLVMModuleRef M, const char *Name);
 
 /**
  * Obtain the named metadata operands for a module.

llvm/trunk/include/llvm-c/Types.h

 /**
  * Represents an LLVM Metadata.
  *
  * This models llvm::Metadata.
  */
 typedef struct LLVMOpaqueMetadata *LLVMMetadataRef;
 
 /**
+ * Represents an LLVM Named Metadata Node.
+ *
+ * This models llvm::NamedMDNode.
+ */
+typedef struct LLVMOpaqueNamedMDNode *LLVMNamedMDNodeRef;
+
+/**
  * Represents an LLVM basic block builder.
  *
  * This models llvm::IRBuilder.
  */
 typedef struct LLVMOpaqueBuilder *LLVMBuilderRef;
 
 /**
  * Represents an LLVM debug info builder.

llvm/trunk/include/llvm/IR/Metadata.h

void replaceUseWith(Metadata *MD) {
     MetadataTracking::untrack(T);
     assert(!Use && "Use is still being tracked despite being untracked!");
   }
 };
 
 //===----------------------------------------------------------------------===//
 /// A tuple of MDNodes.
 ///
-/// Despite its name, a NamedMDNode isn't itself an MDNode. NamedMDNodes belong
-/// to modules, have names, and contain lists of MDNodes.
+/// Despite its name, a NamedMDNode isn't itself an MDNode.
 ///
-/// TODO: Inherit from Metadata.
+/// NamedMDNodes are named module-level entities that contain lists of MDNodes.
+///
+/// It is illegal for a NamedMDNode to appear as an operand of an MDNode.
 class NamedMDNode : public ilist_node<NamedMDNode> {
   friend class LLVMContextImpl;
   friend class Module;
 
   std::string Name;
   Module *Parent = nullptr;
   void *Operands; // SmallVector<TrackingMDRef, 4>
 
public:
   inline iterator_range<op_iterator>  operands() {
     return make_range(op_begin(), op_end());
   }
   inline iterator_range<const_op_iterator> operands() const {
     return make_range(op_begin(), op_end());
   }
 };
 
+// Create wrappers for C Binding types (see CBindingWrapping.h).
+DEFINE_ISA_CONVERSION_FUNCTIONS(NamedMDNode, LLVMNamedMDNodeRef)
+
 } // end namespace llvm
 
 #endif // LLVM_IR_METADATA_H

llvm/trunk/lib/IR/Core.cpp

 
 unsigned LLVMGetMDNodeNumOperands(LLVMValueRef V) {
   auto *MD = cast<MetadataAsValue>(unwrap(V));
   if (isa<ValueAsMetadata>(MD->getMetadata()))
     return 1;
   return cast<MDNode>(MD->getMetadata())->getNumOperands();
 }
 
+LLVMNamedMDNodeRef LLVMGetFirstNamedMetadata(LLVMModuleRef M) {
+  Module *Mod = unwrap(M);
+  Module::named_metadata_iterator I = Mod->named_metadata_begin();
+  if (I == Mod->named_metadata_end())
+    return nullptr;
+  return wrap(&*I);
+}
+
+LLVMNamedMDNodeRef LLVMGetLastNamedMetadata(LLVMModuleRef M) {
+  Module *Mod = unwrap(M);
+  Module::named_metadata_iterator I = Mod->named_metadata_end();
+  if (I == Mod->named_metadata_begin())
+    return nullptr;
+  return wrap(&*--I);
+}
+
+LLVMNamedMDNodeRef LLVMGetNextNamedMetadata(LLVMNamedMDNodeRef NMD) {
+  NamedMDNode *NamedNode = unwrap<NamedMDNode>(NMD);
+  Module::named_metadata_iterator I(NamedNode);
+  if (++I == NamedNode->getParent()->named_metadata_end())
+    return nullptr;
+  return wrap(&*I);
+}
+
+LLVMNamedMDNodeRef LLVMGetPreviousNamedMetadata(LLVMNamedMDNodeRef NMD) {
+  NamedMDNode *NamedNode = unwrap<NamedMDNode>(NMD);
+  Module::named_metadata_iterator I(NamedNode);
+  if (I == NamedNode->getParent()->named_metadata_begin())
+    return nullptr;
+  return wrap(&*--I);
+}
+
+LLVMNamedMDNodeRef LLVMGetNamedMetadata(LLVMModuleRef M,
+                                        const char *Name, size_t NameLen) {
+  return wrap(unwrap(M)->getNamedMetadata(StringRef(Name, NameLen)));
+}
+
+LLVMNamedMDNodeRef LLVMGetOrInsertNamedMetadata(LLVMModuleRef M,
+                                                const char *Name, size_t NameLen) {
+  return wrap(unwrap(M)->getOrInsertNamedMetadata({Name, NameLen}));
+}
+
+const char *LLVMGetNamedMetadataName(LLVMNamedMDNodeRef NMD, size_t *NameLen) {
+  NamedMDNode *NamedNode = unwrap<NamedMDNode>(NMD);
+  *NameLen = NamedNode->getName().size();
+  return NamedNode->getName().data();
+}
+
 void LLVMGetMDNodeOperands(LLVMValueRef V, LLVMValueRef *Dest) {
   auto *MD = cast<MetadataAsValue>(unwrap(V));
   if (auto *MDV = dyn_cast<ValueAsMetadata>(MD->getMetadata())) {
     *Dest = wrap(MDV->getValue());
     return;
   }
   const auto *N = cast<MDNode>(MD->getMetadata());
   const unsigned numOperands = N->getNumOperands();

llvm/trunk/test/Bindings/llvm-c/echo.ll

pad2:
   unreachable
 cleanup:
   %cp = cleanuppad within none []
   cleanupret from %cp unwind to caller
 exit:
   ret void
 }
 
-!llvm.module.flags = !{!1}
+!llvm.module.flags = !{!0}
+!named = !{!1, !2, !3}
 
-!1 = !{i32 2, !"Debug Info Version", i32 3}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !{}
+!2 = distinct !{}
+!3 = !{!4}
+!4 = distinct !{}

llvm/trunk/tools/llvm-c-test/echo.cpp

FunDecl:
   }
 
 AliasDecl:
   Begin = LLVMGetFirstGlobalAlias(Src);
   End = LLVMGetLastGlobalAlias(Src);
   if (!Begin) {
     if (End != nullptr)
       report_fatal_error("Range has an end but no beginning");
-    return;
+    goto NamedMDDecl;
   }
 
   Cur = Begin;
   Next = nullptr;
   while (true) {
     size_t NameLen;
     const char *Name = LLVMGetValueName2(Cur, &NameLen);
     if (LLVMGetNamedGlobalAlias(M, Name, NameLen))
while (true) {
     }
 
     LLVMValueRef Prev = LLVMGetPreviousGlobalAlias(Next);
     if (Prev != Cur)
       report_fatal_error("Next.Previous global is not Current");
 
     Cur = Next;
   }
+
+NamedMDDecl:
+  LLVMNamedMDNodeRef BeginMD = LLVMGetFirstNamedMetadata(Src);
+  LLVMNamedMDNodeRef EndMD = LLVMGetLastNamedMetadata(Src);
+  if (!BeginMD) {
+    if (EndMD != nullptr)
+      report_fatal_error("Range has an end but no beginning");
+    return;
+  }
+
+  LLVMNamedMDNodeRef CurMD = BeginMD;
+  LLVMNamedMDNodeRef NextMD = nullptr;
+  while (true) {
+    size_t NameLen;
+    const char *Name = LLVMGetNamedMetadataName(CurMD, &NameLen);
+    if (LLVMGetNamedMetadata(M, Name, NameLen))
+      report_fatal_error("Named Metadata Node already cloned");
+    LLVMGetOrInsertNamedMetadata(M, Name, NameLen);
+
+    NextMD = LLVMGetNextNamedMetadata(CurMD);
+    if (NextMD == nullptr) {
+      if (CurMD != EndMD)
+        report_fatal_error("");
+      break;
+    }
+
+    LLVMNamedMDNodeRef PrevMD = LLVMGetPreviousNamedMetadata(NextMD);
+    if (PrevMD != CurMD)
+      report_fatal_error("Next.Previous global is not Current");
+
+    CurMD = NextMD;
+  }
 }
 
 static void clone_symbols(LLVMModuleRef Src, LLVMModuleRef M) {
   LLVMValueRef Begin = LLVMGetFirstGlobal(Src);
   LLVMValueRef End = LLVMGetLastGlobal(Src);
 
   LLVMValueRef Cur = Begin;
   LLVMValueRef Next = nullptr;
FunClone:
   }
 
 AliasClone:
   Begin = LLVMGetFirstGlobalAlias(Src);
   End = LLVMGetLastGlobalAlias(Src);
   if (!Begin) {
     if (End != nullptr)
       report_fatal_error("Range has an end but no beginning");
-    return;
+    goto NamedMDClone;
   }
 
   Cur = Begin;
   Next = nullptr;
   while (true) {
     size_t NameLen;
     const char *Name = LLVMGetValueName2(Cur, &NameLen);
     LLVMValueRef Alias = LLVMGetNamedGlobalAlias(M, Name, NameLen);
while (true) {
     }
 
     LLVMValueRef Prev = LLVMGetPreviousGlobalAlias(Next);
     if (Prev != Cur)
       report_fatal_error("Next.Previous global alias is not Current");
 
     Cur = Next;
   }
+
+NamedMDClone:
+  LLVMNamedMDNodeRef BeginMD = LLVMGetFirstNamedMetadata(Src);
+  LLVMNamedMDNodeRef EndMD = LLVMGetLastNamedMetadata(Src);
+  if (!BeginMD) {
+    if (EndMD != nullptr)
+      report_fatal_error("Range has an end but no beginning");
+    return;
+  }
+
+  LLVMNamedMDNodeRef CurMD = BeginMD;
+  LLVMNamedMDNodeRef NextMD = nullptr;
+  while (true) {
+    size_t NameLen;
+    const char *Name = LLVMGetNamedMetadataName(CurMD, &NameLen);
+    LLVMNamedMDNodeRef NamedMD = LLVMGetNamedMetadata(M, Name, NameLen);
+    if (!NamedMD)
+      report_fatal_error("Named MD Node must have been declared already");
+
+    unsigned OperandCount = LLVMGetNamedMetadataNumOperands(Src, Name);
+    LLVMValueRef *OperandBuf = static_cast<LLVMValueRef *>(
+              safe_malloc(OperandCount * sizeof(LLVMValueRef)));
+    LLVMGetNamedMetadataOperands(Src, Name, OperandBuf);
+    for (unsigned i = 0, e = OperandCount; i != e; ++i) {
+      LLVMAddNamedMetadataOperand(M, Name, OperandBuf[i]);
+    }
+    free(OperandBuf);
+
+    NextMD = LLVMGetNextNamedMetadata(CurMD);
+    if (NextMD == nullptr) {
+      if (CurMD != EndMD)
+        report_fatal_error("Last Named MD Node does not match End");
+      break;
+    }
+
+    LLVMNamedMDNodeRef PrevMD = LLVMGetPreviousNamedMetadata(NextMD);
+    if (PrevMD != CurMD)
+      report_fatal_error("Next.Previous Named MD Node is not Current");
+
+    CurMD = NextMD;
+  }
 }
 
 int llvm_echo(void) {
   LLVMEnablePrettyStackTrace();
 
   LLVMModuleRef Src = llvm_load_module(false, true);
   size_t SourceFileLen;
   const char *SourceFileName = LLVMGetSourceFileName(Src, &SourceFileLen);
   size_t ModuleIdentLen;
   const char *ModuleName = LLVMGetModuleIdentifier(Src, &ModuleIdentLen);
   LLVMContextRef Ctx = LLVMContextCreate();
   LLVMModuleRef M = LLVMModuleCreateWithNameInContext(ModuleName, Ctx);
 
   LLVMSetSourceFileName(M, SourceFileName, SourceFileLen);
   LLVMSetModuleIdentifier(M, ModuleName, ModuleIdentLen);
 
-  size_t SourceFlagsLen;
-  LLVMModuleFlagEntry *ModuleFlags =
-      LLVMCopyModuleFlagsMetadata(Src, &SourceFlagsLen);
-  for (unsigned i = 0; i < SourceFlagsLen; ++i) {
-    size_t EntryNameLen;
-    const char *EntryName =
-        LLVMModuleFlagEntriesGetKey(ModuleFlags, i, &EntryNameLen);
-    LLVMAddModuleFlag(M, LLVMModuleFlagEntriesGetFlagBehavior(ModuleFlags, i),
-                      EntryName, EntryNameLen,
-                      LLVMModuleFlagEntriesGetMetadata(ModuleFlags, i));
-  }
-
   LLVMSetTarget(M, LLVMGetTarget(Src));
   LLVMSetModuleDataLayout(M, LLVMGetModuleDataLayout(Src));
   if (strcmp(LLVMGetDataLayoutStr(M), LLVMGetDataLayoutStr(Src)))
     report_fatal_error("Inconsistent DataLayout string representation");
 
   size_t ModuleInlineAsmLen;
   const char *ModuleAsm = LLVMGetModuleInlineAsm(Src, &ModuleInlineAsmLen);
   LLVMSetModuleInlineAsm2(M, ModuleAsm, ModuleInlineAsmLen);
 
   declare_symbols(Src, M);
   clone_symbols(Src, M);
   char *Str = LLVMPrintModuleToString(M);
   fputs(Str, stdout);
 
-  LLVMDisposeModuleFlagsMetadata(ModuleFlags);
   LLVMDisposeMessage(Str);
   LLVMDisposeModule(Src);
   LLVMDisposeModule(M);
   LLVMContextDispose(Ctx);
 
   return 0;
 }