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

David Blaikie via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 21 09:01:47 PST 2017

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?


More information about the llvm-commits mailing list