[PATCH] D143967: [DebugInfo][BPF] Add 'btf:type_tag' annotation in DWARF

[David Faust via Phabricator via cfe-commits]

dfaust added a comment.

In D143967#4231911 <https://reviews.llvm.org/D143967#4231911>, @jemarch wrote:

>> eddyz87 added a subscriber: anakryiko.
>> eddyz87 added a comment.
>>
>> In D143967#4231746 <https://reviews.llvm.org/D143967#4231746> https://reviews.llvm.org/D143967#4231746, @jemarch wrote:
>>
>>>> eddyz87 added a comment.
>>>> ...
>>>> If we consider type tag a qualifier, conceptually it would be simpler
>>>> if ordering would not matter for it as well, so that the following
>>>> have identical meaning:
>>>>
>>>> - `__tag volatile int`
>>>> - `volatile __tag int`
>>>
>>> Makes sense.
>>>
>>> But then I think we should allow the tags to be anywhere in the chain in
>>> the DWARF, and not necessarily in the qualified type.  For example
>>> given:
>>>
>>>   typedef const int bar;
>>>   volatile bar __tag1 foo;
>>>
>>> The resulting DWARF type chain is:
>>>
>>>   volatile -> typedef (bar) -> const -> int
>>>
>>> IMO we should allow __tag1 to be a child of any of the elements of the
>>> chain, like:
>>>
>>>   volatile -> typedef (bar) -> const -> int
>>>                    |
>>>                  __tag1
>>>
>>> or
>>>
>>>   volatile -> typedef (bar) -> const -> int
>>>                                          |
>>>                                       __tag1
>>>
>>> or
>>>
>>>   volatile -> typedef (bar) -> const -> int
>>>                                  |
>>>                               __tag1
>>>
>>> or
>>>
>>>   volatile -> typedef (bar) -> const -> int
>>>      |
>>>    __tag1
>>>
>>> would be all accepted and equivalent.  Also Faust noted that GCC
>>> sometimes optimizes out the typedef nodes, so we need to be able to
>>> place the tags (in the internal representatinos) in the typedef'ed type
>>> instead.
>>
>> Well, it's a logical consequence, I guess.
>> In practice I'm going to emit it like below:
>>
>>   volatile -> const -> int
>>                         |
>>                      __tag1
>
> We can probably do the same in GCC, to be consistent.
>
>> But please remember that in BTF it would have to be encoded like this:
>>
>>   __tag1 -> volatile -> const -> int
>>
>> (that's how kernel expects it, we can change the kernel but will loose
>> backwards compatibility).  For DWARF -> BTF transformation in `pahole`
>> I will make the necessary modifications, but BTF might also be emitted
>> C->BPF backend.
>
> We can do the same thing when we generate BTF directly from GCC.
> Faust, do you agree with the above?

OK, that's fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967