<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 21, 2017 at 9:46 AM Paul Robinson via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">probinson added a comment.<br class="gmail_msg">
<br class="gmail_msg">
In <a href="https://reviews.llvm.org/D30206#682359" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D30206#682359</a>, @dblaikie wrote:<br class="gmail_msg">
<br class="gmail_msg">
> (is the DwarfGenerator change tested at all? I mean it's a test utility, so maybe that's overkill, not sure)<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
It's like this: I was originally going to avoid the llvm-dwarfdump changes in the first patch, but it turns out there are a couple of tests that specify version 5 in the IR and use llvm-dwarfdump, so I had to make the DebugInfo lib changes. Having done that, the unit tests failed because the generator was not producing the new format. So I had to fix the generator.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: include/llvm/Support/Dwarf.def:806<br class="gmail_msg">
+HANDLE_DW_UT(0x05, split_compile)<br class="gmail_msg">
+HANDLE_DW_UT(0x06, split_type)<br class="gmail_msg">
<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> 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)?<br class="gmail_msg">
These are the unit types defined in the spec. Split and non-split type units are nearly identical, the difference being that a split type unit can't use DW_AT_str_offsets_base, and of course where the units actually go.<br class="gmail_msg"></blockquote><div><br></div><div>Oh, right, sorry - got confused. I was worried about /skeleton/ type units, but I see they're not there (there's just "skeleton" which is a skeleton CU, I assume).</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:693-694<br class="gmail_msg">
<br class="gmail_msg">
- DwarfUnit::emitHeader(UseOffsets);<br class="gmail_msg">
+ DwarfUnit::emitCommonHeader(UseOffsets, Skeleton ? dwarf::DW_UT_skeleton<br class="gmail_msg">
+ : dwarf::DW_UT_compile);<br class="gmail_msg">
}<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> 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?<br class="gmail_msg">
So, you think I'm missing a split-dwarf test that uses v5? I'll look into this.<br class="gmail_msg"></blockquote><div><br></div><div>Yep</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: lib/DebugInfo/DWARF/DWARFUnit.cpp:99<br class="gmail_msg">
+ AddrSize = debug_info.getU8(offset_ptr);<br class="gmail_msg">
+ }<br class="gmail_msg">
if (IndexEntry) {<br class="gmail_msg">
----------------<br class="gmail_msg">
aprantl wrote:<br class="gmail_msg">
> This should probably be tested by a llvm-dwarfdump test?<br class="gmail_msg">
Are there llvm-dwarfdump tests?</blockquote><div><br></div><div>There are some - using checked in binary/object files.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I couldn't find any. There is a test/tools/llvm-dwarfdump directory (with AArch64 and ARM subdirectories) but not actual files.<br class="gmail_msg"></blockquote><div><br></div><div>There are some in test/DebugInfo/dwarfdump* I think.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">This code was needed because there are a couple of debug-info tests that specify version 5 in the IR, and then use llvm-dwarfdump in their RUN lines. Having done this change, I had to fix the unit-test DWARF generator because the unit tests were failing.</blockquote><div><br></div><div>What was it that caused breakage in the DwarfGenerator once llvm-dwarfdump had v5 support?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> That was plenty to convince me that things were working.<br class="gmail_msg">
<br class="gmail_msg">
If you want a separate llvm-dwarfdump test as well, I can do that.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D30206" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D30206</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>