[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#2851926 <https://reviews.llvm.org/D105251#2851926>, @davidxl wrote:

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

Thanks for clarifying!

I did some archival to confirm: yes, this was intentional. The change for globals landed in: https://reviews.llvm.org/D20414

> IR: Allow multiple global metadata attachments with the same type.

(although it missed documenting the difference in LangRef, certainly it was specifically changing globals without changing instructions)

I disagree with the assertion in the commit message:

> This will be necessary to allow the global merge pass to attach
> multiple debug info metadata nodes to global variables once we reverse
> the edge from DIGlobalVariable to GlobalVariable.

(since it wasn't necessary... multiple attachments could have been handled by adding indirection, as is typical for instruction metadata)
but the context/impact was different since the commit landed around the same time that global metadata attachments were introduced (https://reviews.llvm.org/D20074). This wasn't a subtle behaviour change that could affect existing code handling metadata attachments for global variables, since there was no such code (yet).

(The inconsistency between instructions / globals is certainly unfortunate... just want to make sure that if we add this feature for instructions the impact is carefully considered...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105251