[PATCH] D115325: [DWARF] Fix PR51087 Extraneous enum record in DWARF with type units

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 13 16:29:14 PST 2021


dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

In D115325#3188672 <https://reviews.llvm.org/D115325#3188672>, @Orlando wrote:

> Thanks for taking a look at this.
>
> In D115325#3186306 <https://reviews.llvm.org/D115325#3186306>, @dblaikie wrote:
>
>> under what conditions do we get this redundant pointer type in the CU you're referring to here?
>
> Here's an example with target `x86_64-unknown-linux-gnu` using clang built at 51dc466642c5 <https://reviews.llvm.org/rG51dc466642c5566c289468b269a8c69b0e447720>.
>
>   $ cat test.cpp
>   struct O { struct S { int x; } y; };
>   int f(void *p) {
>     return static_cast<O::S*>(p)->x;
>   }
>   
>   $ clang++ test.cpp -c -gdwarf-5 -fdebug-types-section -o - | llvm-dwarfdump - -o-
>   <snip>
>   0x0000003f:   DW_TAG_pointer_type
>                   DW_AT_type	(0x0000004d "O::S")
>   
>   0x00000044:   DW_TAG_structure_type
>                   DW_AT_declaration	(true)
>                   DW_AT_signature	(0x9ff533511b48ced2)
>   
>   0x0000004d:     DW_TAG_structure_type
>                     DW_AT_declaration	(true)
>                     DW_AT_signature	(0x7b36f109e857d200)
>   <snip>
>
> DIE `0x0000003f` is not referenced in the CU. The CU DIE is generated because it's a "retained type" (in `CUNode->getRetainedTypes()`). It's still emitted if even when we apply the fix in this patch to retained types because `DW_TAG_pointer_type` types don't get type units (in `DwarfUnit::createTypeDIE(const DIEScope*, DIE&, const DIType*)`. So, my thinking was that consumers probably don't need the `DW_TAG_pointer_type` DIE to interpret pointers of the derived type; as long as we have the derived type DIE (in a type unit) then that should be enough? I don't have 100% confidence in that assumption though - what do you think?

Yep, that sounds right to me.

So we retain types that we know are used in ways that aren't otherwise tied to the rest of the DWARF graph - if you think about DWARF nodes/DIEs as a reachability graph, starting from functions and variables. Those functions and variables exist in the llvm::Module and reference the DI* description of the function or variable - those DI* descriptions reference types, types reference other types, etc. But if there's never a function or variable that references a type it might be skipped entirely - but in some syntactic constructs/cases, it's important that it isn't skipped - one of the canonical examples of that is where the type is only ever used within an expression and never as the type of a used and named variable.

We probably should only put user defined types in the retained types list - but that would require decomposing non-user-defined types in the frontend (eg: if you have a "void (*)(T1, T2)" type retained - we only need to retain T1 and T2, not the function pointer type wrapped around it (it's reasonable to expect a DWARF consumer to be able to build any non-user-defined type - even ones that that never appeared in the source program (so maybe a user ends up needing "T1 (*)(T2)" in their debugging context, the DWARF consumer would be able to create that - there's no need for us to encode that type or any other, so long as we provide T1 and T2)

But I guess for now we could apply the same logic to enums and to retained types, with one minor change to use dyn_cast instead of cast, in the retained types version - so if it is a top level user defined type in the retained types list, it'd skip the skeleton type DIE in that case. In other cases it'd still end up with the unnecessary poinetr type, function type, whatever.

> I'm not sure how often this actually comes up in practice - I only noticed it when writing a test for applying this change to "retained types" too (the part that you have also highlighted with an inline comment). I am not sure of all the ways that a type becomes a "retained type"; the source above is inspired by `llvm/test/DebugInfo/COFF/retained-types.ll`. I decided to leave the retained types alone in this patch because I felt I didn't fully understand what causes them, what they look like "on average", and how many useless DIEs we really emit because of them. i.e. if retained types are always or mostly pointers then we'd need to apply the change described above (if it's possible) first to see any wins. Do you have any insight on this? Otherwise I can look into this further.




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

https://reviews.llvm.org/D115325



More information about the llvm-commits mailing list