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

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 added a subscriber: anakryiko.
eddyz87 added a comment.

In 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

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.

@yonghong-song, @anakryiko, what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967