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

[Eduard Zingerman via Phabricator via cfe-commits]

eddyz87 added a comment.

In D143967#4438815 <https://reviews.llvm.org/D143967#4438815>, @dblaikie wrote:

> I haven't looked closely at this, but from a vague/quick reading, it sounds like this is creating annotations on certain type DINodes, but then moving them around to different types? It'd be nice if we could avoid that/create them in the right place in the first place.

Hi @dblaikie, thank you for taking a look!

I assume you refer to the "placeholder" nodes created for annotated types.
I added these to handle the following example:

  #define __tag1 __attribute__((btf_type_tag("tag1")))
  
  struct st {
    struct st __tag1 *self;
  } g;

Here I need to create two `DICompositeType` instances for `struct st`:

- one with `btf_type_tag` annotations;
- one without `btf_type_tag` annotations.

The AST for `struct st __tag1 *` looks as follows:

  PointerType 0x55c364ead270 'struct st  __attribute__((btf_type_tag("tag1")))*'
  `-BTFTagAttributedType 0x55c364ead210
        'struct st __attribute__((btf_type_tag("tag1")))' sugar
    `-ElaboratedType 0x55c364ead1a0 'struct st' sugar
      `-RecordType 0x55c364ead120 'struct st'
        `-Record 0x55c364ead0a0 'st'

(Note the `BTFTagAttributedType` node).

W/o type tags this examples is processed as follows:

- `getOrCreateType()` is called for `struct st`
  - `CreateType(const RecordType *)` is called for `struct st`
    - `CreateTypeDefinition(const RecordType *)` is called for `struct st`
      - `getOrCreateLimitedType(const RecordType *)` is called for `struct st`
        - `CreateLimitedType(const RecordType *)` is called for `struct st`, which returns a `TempDICompositeType` (let's call it `T`)
        - `TypeCache` for `struct st` is set to `T`
      - members of `struct st` are processed:
        - `getOrCreateType()` is called for `struct st *`
          - `CreateType(const PointerType *)` is called for `struct st *`
            - `getOrCreateType()` is called for `struct st`, which returns `T`, because of `TypeCache` state.
      - all uses of `T` are replaced by complete type.

With type tags processing is similar, but pointee type for `struct st *` is actually `(BTFTagAttributedType (RecordType 'struct st'))`.
The `CreateType(const BTFTagAttributedType *)` creates an instance of `TempDIDerivedType` with correct annotations (the placeholder) and base type corresponding to `struct st`.
Then, at `CGDebugInfo::finalize()` the placeholder is removed:

- the copy of the base type with correct annotations is created;
- all used of placeholder are replaced by this copy.

So, because at the point when `CreateType(BTFTagAttributedType *)` is called, the base type is not guaranteed to be complete yet there is a need to create some kind of temporary node.

An alternative to my current implementation would be creation of `TempDICompositeType` nodes, but this would require passing annotations down from `CreateType(BTFTagAttributedType *)` to `CreateLimitedType(const RecordType *)` via `getOrCreateType()` / `CreateTypeNode()`. But this seems very intrusive for the sake of a narrow feature used by a single backend.

Thus, I think that current implementation is better, because all `btf_decl_tag` annotations processing is grouped in two points:

- `CreateType(const BTFTagAttributedType *)`;
- `copyAnnotations()` called from `finalize()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143967