[PATCH] D118416: [Metadata] Use temporary MD nodes when appending module flags during module linking

Tue Feb 1 18:12:25 PST 2022

dblaikie added a comment.

In D118416#3289247 <https://reviews.llvm.org/D118416#3289247>, @dexonsmith wrote:

> In D118416#3286076 <https://reviews.llvm.org/D118416#3286076>, @dblaikie wrote:
>
>>> IMO, the first approach I mention above would be best (possibly with the second approach as a follow up!) since the resize() operation would be generally useful; there are lots of metadata nodes that are really just lists of arbitrary size, and that get extended later on (e.g., @dblaikie @aprantl, I think there are places in debug info IR where new nodes are allocated for this sort of thing, is that right?).
>>
>> I'm actually not sure how many debug info nodes there are that get incrementally appended to... - the CU list itself, yes, but otherwise mostly the lists are made by frontends and made the right size the first time, I think? yeah, DIBuilder keeping a SmallVector of retained type nodes, then creating the necessary vector to create an MDNode from that in one shot, without incrementally appending.
>>
>> But I could be wrong - just my rough estimate/recollection.
>
> Yeah, I think it was the CU list (hits this kind of problem in LTO, probably has some custom code now) and the retained types (delays creation in IRBuilder to work around this) that I was thinking of.
>
> Since `distinct` and `temporary` metadata are non-const anyway, it seems better to me to allow the size to change dynamically, and teaching the clients to take advantage of that rather than having extra side data, or temporarily invalid state (note that the verifier fails if it finds a temporary node).

Sounds OK to me, if it makes sense to you!

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

https://reviews.llvm.org/D118416