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

David Li via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 30 18:51:03 PDT 2021


davidxl added a comment.

In D105251#2851896 <https://reviews.llvm.org/D105251#2851896>, @dexonsmith wrote:

> I'm a bit concerned about the subtle difference in behaviour... in the past, metadata schemes that needed multiple attachments added a level of indirection (point at a tuple of metadata). Will that work for the heap profiler?
>
>> because it is an inconsistency in IR support
>> (the LLVM langref does not specify anything with regards to this
>> behavior).
>
> While the current behaviour (one attachment per kind) is not documented in LangRef, it's documented in the header docs, and I do think it was intentional; and in any case, it has been around for a long time and there could be code patterns relying on it.

I think Teresa referred to the inconsistency between Global Object and Instruction. Is that intentional?

> Since this changes IR semantics, I suggest adding a change to LangRef and a discussion on llvm-dev (and/or the new proposals system).
>
> A few design points that I'm not sure about:
>
> - What should `getMetadata` do if there are multiple attachments for the requested kind? (assert? return nullptr? return the first, last, or arbitrary?)
> - Should there be a way for metadata kinds to explicitly opt-in (or out) of allowing multiple attachments? (Maybe could be stored on the LLVMContext somehow?)
>   - If not, what do we do about the inconsistency with `!dbg`? Support multiple attachments somehow?
> - (And: why is this better than using a level of indirection when multiple attachments are needed?)
>
> I also have a bunch of comments inline assuming this goes forward.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105251



More information about the llvm-commits mailing list