[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D111199#3099988 <https://reviews.llvm.org/D111199#3099988>, @aaron.ballman wrote:

> In D111199#3097692 <https://reviews.llvm.org/D111199#3097692>, @dblaikie wrote:
>
>> In D111199#3096124 <https://reviews.llvm.org/D111199#3096124>, @aaron.ballman wrote:
>>
>>> In D111199#3095623 <https://reviews.llvm.org/D111199#3095623>, @yonghong-song wrote:
>>>
>>>>> Ah, yeah, I see what you mean - that does seem sort of unfortunate. Is it possible these attributes could only appear on typedefs and they'd be more readily carried through that without needing extra typeloc tracking? (sorry for not having read back through the rest of the review - which may've started there and ended up here as a more general form of the attribute?)
>>>>
>>>> For the question, "is it possible these attributes could only appear on typedefs?" The answer is "not really possible". We are targeting existing linux kernel where existing type attributes (__user, __rcu, ...) have been used in places other than typedef quite extensively (e.g., function argument type, function return type, field type, etc.).
>>>>
>>>> In one of my earlier prototypes, I put the tag string itself in AttributedType and with this we can avoid TypeLoc, but I understand this is not conventional usage and that is why we do through TypeLoc mechanism. @aaron.ballman can further comment on this.
>>>
>>> FWIW, I made that request because AttributedTypeLoc stores the Attr * for the attributed type, so we can get the attribute argument information from that rather than having to duplicate it within a new TypeLoc object.
>>
>> So could the debug info code be done with AttributedType (I guess currently CGDebugInfo skips over those - but we could not skip over them and check if it's one of these attributes, otherwise skip) rather than passing around the extra TypeLoc?
>
> That's my hope, but I wasn't certain if there were type modifications happening where we need to calculate a `TypeLoc` that's different from what we would get out of the type itself. I would expect that doesn't happen during CodeGen (seems awfully late to be changing types), but wasn't 100% sure.

Hmm - not sure I really follow. Is there a greater risk of this than with other types we're emitting/lrelying entirely on types and not TypeLocs already? Any way we can validate whether this is an issue/is necessary to handle?

Otherwise I'd be inclined to go with the pure Type based solution, maybe leave a "watch out for bugs related to type changes" comment somewhere in case bugs do come up in the future?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111199