[PATCH] D93400: [IRBuilder] Generalize debug loc handling for arbitrary metadata.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/include/llvm/IR/IRBuilder.h:212-219
+  /// Collect metadata with IDs \p MetadataKinds from \p Src which should be
+  /// added to all created instructions.
+  void CollectMetadataToCopy(Instruction *Src,
+                             ArrayRef<unsigned> MetadataKinds) {
+    for (unsigned K : MetadataKinds)
+      if (auto *MD = Src->getMetadata(K))
+        AddOrRemoveMetadataToCopy(K, MD);
----------------
lebedev.ri wrote:
> Don't you have an invalidation issue here?
> What happens if we first call `CollectMetadataToCopy()` on an instruction that had metadata `zz`,
> and then call it again on an instruction that didn't have that metadata? (the instcombine's pattern)
Yeah, originally I thought of collecting metadata to strictly increase the metadata to copy. But this is not in line with current practices for DebugLoc, where setting an empty DebugLoc removes the DebugLoc. I think in light of that, it makes sense for `CollectMetadataToCopy` to also remove entries from `MetadataTocopy`, if they are not present on `Src`. Also updated the comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D93400/new/

https://reviews.llvm.org/D93400