[PATCH] D20414: IR: Allow multiple global metadata attachments with the same type.

Peter Collingbourne via llvm-commits llvm-commits at lists.llvm.org
Fri May 20 13:04:08 PDT 2016


pcc added inline comments.

================
Comment at: lib/IR/Metadata.cpp:1159
@@ +1158,3 @@
+  // need to preserve the original insertion order though.
+  if (Result.size() > 1)
+    std::stable_sort(Result.begin(), Result.end(),
----------------
aprantl wrote:
> Why is this check necessary?
It isn't, removed.

================
Comment at: lib/IR/Metadata.cpp:1338
@@ -1297,2 +1337,3 @@
+void GlobalObject::addMetadata(unsigned KindID, MDNode *MD) {
   if (MD) {
     if (!hasMetadata())
----------------
aprantl wrote:
> ```
> if (!MD)
>   return;
> ```
Rethinking this, maybe this function should always expect an non-null MDNode, since unlike `setMetadata` the intent is always to add an attachment. WDYT?

================
Comment at: lib/IR/Metadata.cpp:1382
@@ +1381,3 @@
+  eraseMetadata(KindID);
+  if (N)
+    addMetadata(KindID, N);
----------------
aprantl wrote:
> This condition is redundant since addMetadata will check for a nullptr.
See above

================
Comment at: lib/IR/Verifier.cpp:2002
@@ -2000,1 +2001,3 @@
       case LLVMContext::MD_dbg:
+        AssertDI(++NumDebugAttachments <= 1,
+                 "function must have a single !dbg attachment", &F, I.second);
----------------
aprantl wrote:
> ```
> ++NumDebugAttachments <= 1
> ```
> can never be <1, right?
No, changed to ==


http://reviews.llvm.org/D20414





More information about the llvm-commits mailing list