[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?
https://reviews.llvm.org/D30206
More information about the llvm-commits
mailing list