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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri 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)
Actually, my suggestion is still incorrect, because it still doesn't drop metadata
You want something like
```
  void CollectMetadataToCopy(Instruction *Src,
                             ArrayRef<unsigned> MetadataKinds) {
    // First, refresh (either update or delete) all the existing metadata
    for (unsigned K : make_first_range(MetadataToCopy))
      AddOrRemoveMetadataToCopy(K, Src->getMetadata(K));
    // Then, add new metadata (either add or update)
    for (unsigned K : MetadataKinds)
      AddOrRemoveMetadataToCopy(K, Src->getMetadata(K));
  }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93400