[PATCH] D30206: [DWARF5] Emit new unit header

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

Haven't actually read the spec to confirm this is implementing things correctly - few comments on the code, and looks like it could use some more test coverage. (is the DwarfGenerator change tested at all? I mean it's a test utility, so maybe that's overkill, not sure)



================
Comment at: include/llvm/Support/Dwarf.def:806
+HANDLE_DW_UT(0x05, split_compile)
+HANDLE_DW_UT(0x06, split_type)
 
----------------
Guess I should've read the DWARF5 spec. Why is there a split type unit? I remember Cary getting rid of this shortly after the first prototype of Fission. I'm asuming it's at least not required (ie: it's optional)?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:693-694
 
-  DwarfUnit::emitHeader(UseOffsets);
+  DwarfUnit::emitCommonHeader(UseOffsets, Skeleton ? dwarf::DW_UT_skeleton
+                                                   : dwarf::DW_UT_compile);
 }
----------------
This is probably backwards - "Skeleton" is a pointer to the skeleton (so it implies this unit is /not/ a skeleton). Test coverage should demonstrate/confirm this?


https://reviews.llvm.org/D30206