[PATCH] D70880: [DWARF5][Debuginfo] Not matching compilation unit type (DW_UT_skeleton) and root DIE (DW_TAG_compile_unit)

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D70880#1767924 <https://reviews.llvm.org/D70880#1767924>, @avl wrote:

> > @avl - are you planning to do the DW_TAG_split_compile/DW_TAG_split_type tags required by DWARFv5 too in separate patches?
>
> My understanding is that everything is already done here. There is no DW_TAG_split_compile/DW_TAG_split_type tags. There should be used DW_TAG_compile_unit+DW_UT_split_compile and DW_TAG_type_unit+DW_UT_split_type. It looks like these already are set correctly.


Ah, right you are - my mistake!



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfFile.h:106-109
+  /// That value indicates whether we are generating file
+  /// containing skeleton compilation units.
+  bool IsSkeleton = false;
+
----------------
avl wrote:
> dblaikie wrote:
> > Rather than adding this field - I think it'd be cleaner to pass the boolean (maybe use an `enum class UnitKind { Skeleton, Full }` to make it more readable) into the DwarfCompileUnit ctor directly. The two callers know statically which kind of unit they are constructing, so there would be no need for branching/conditionals/dynamic computation of the unit type by doing it that way.
> > 
> > (& probably add it as a default argument ("Full") to the end, then only the Skeleton unit construction gets the extra argument)
> I did not clearly get the idea... 
> 
> 1. add a parameter of enum class UnitKind to DwarfCompileUnit ctor.
> 
> 2. add a method like this into  DwarfFile:
> 
> 
> ```
> UnitKind DwarfFile::GetCompileUnitType(){
>   if (Asm->getDwarfDebug()->getDwarfVersion() >= 5 && IsSkeleton)
>     return UnitKind::Skeleton;
> 
>   return UnitKind::Full;
> } 
> ```
> 
> 3. call GetCompileUnitType when creating DwarfCompileUnit.
> 
> is that correct?
Not quite what I had in mind

  1. - yep, add the extra (let's say defaulted) DwarfCompileUnit ctor parameter
  2. pass the Skeleton UnitKind to the DwarfCompileUnit construction in DwarfDebug::constructSkeletonCU

& then in the implementation of (1) I guess something like:

 DwarfUnit(Kind == UnitKind::Skeleton && Asm->getDwarfDebug()->getDwarfVersion() >= 5 ? dwarf::DW_TAG_skeleton_unit : dwarf::DW_TAG_compile_unit, Node, A, DW, DWU)

(or a utility function to pick between the two unit tags in the same way, if you like - but not sure it's worth it)


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

https://reviews.llvm.org/D70880