<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 21, 2017, at 9:55 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Feb 21, 2017 at 9:46 AM Paul Robinson via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></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 class=""><br class=""></div><div class="">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 class=""> </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 class=""><br class=""></div><div class="">Yep</div><div class=""> </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 class=""><br class=""></div><div class="">There are some - using checked in binary/object files.</div></div></div></div></blockquote><div><br class=""></div><div>I don't think we need to go that far. A round-trip test would be good enough, since the testcase included in this review already checks for the correct binary output format of the DWARF generator.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </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 class=""><br class=""></div><div class="">There are some in test/DebugInfo/dwarfdump* I think.</div></div></div></div></blockquote><div><br class=""></div><div>And if there aren't any, we should create that directory to encourage future use :-)</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_quote"><div class=""> </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 class=""><br class=""></div><div class="">What was it that caused breakage in the DwarfGenerator once llvm-dwarfdump had v5 support?</div><div class=""> </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>
</div></blockquote></div><br class=""></body></html>