[PATCH] D105251: [IR] Allow multiple instruction metadata attachments with same type

[Duncan P. N. Exon Smith via Phabricator via llvm-commits]

dexonsmith added a comment.

In D105251#2855987 <https://reviews.llvm.org/D105251#2855987>, @tejohnson wrote:

> 1. My primary motivation: allow multiple new heap prof metadatas on a single allocation call without indirection, via multiple !prof attachments each with the same tag. E.g.
>
>   call malloc !prof !0, !prof !1
>   
>   !0 = !{"heap", [stack trace 1 heap profile data]}
>   !1 = !{"heap", [stack trace 2 heap profile data]} 

Ah, I'd completely missed this, I was just thinking about the orthogonal metadata case.

Seems like a separate `!heapprof` attachment that uses the feature from this patch (leaving `!prof` alone, limited by the verifier to 1-per-instruction) could be pretty clean, but as you say, there are a number of reasonable options.

> Agreed. Let me discuss this with my collaborators and think about it a little more. But yes, in either case let's plan to update LangRef. I was surprised that it didn't mention anything in this regard. Will circle back probably after the holiday.

Sounds great!



================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:5526-5528
+    SmallVector<MDNode *, 1> MDs;
+    I.getMetadata(LLVMContext::MD_prof, MDs);
+    for (MDNode *MD : MDs) {
----------------
Just noticed this: if this patch does move forward, I'd suggest deferring this change for a later patch and making this one just about the IR feature. Maybe it could be tested with a kind that doesn't have a fixed id, like `!attachment`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105251