Agreed, it'll be fine.<br><br><div>-eric</div><br><div class="gmail_quote">On Wed Nov 12 2014 at 5:32:41 PM Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Nov 12, 2014, at 4:40 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Wed, Nov 12, 2014 at 3:48 PM, Frederic Riss<span> </span><span dir="ltr"><<a href="mailto:friss@apple.com" target="_blank">friss@apple.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: friss<br>Date: Wed Nov 12 17:48:14 2014<br>New Revision: 221837<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=221837&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=221837&view=rev</a><br>Log:<br>Fix emission of Dwarf accelerator table when there are multiple CUs.<br><br>The DIE offset in the accel tables is an offset relative to the start<br>of the debug_info section, but we were encoding the offset to the<br>start of the containing CU.<br><br>Modified:<br> <span> </span>llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br> <span> </span>llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h<br> <span> </span>llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br> <span> </span>llvm/trunk/test/DebugInfo/cross-cu-inlining.ll<br><br>Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=221837&r1=221836&r2=221837&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=221837&r1=221836&r2=221837&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp Wed Nov 12 17:48:14 2014<br>@@ -13,6 +13,7 @@<br><br> #include "DwarfAccelTable.h"<br> #include "DIE.h"<br>+#include "DwarfCompileUnit.h"<br> #include "DwarfDebug.h"<br> #include "llvm/ADT/STLExtras.h"<br> #include "llvm/ADT/Twine.h"<br>@@ -174,7 +175,7 @@ void DwarfAccelTable::EmitOffsets(AsmPri<br> // Walk through the buckets and emit the full data for each element in<br> // the bucket. For the string case emit the dies and the various offsets.<br> // Terminate each HashData bucket with 0.<br>-void DwarfAccelTable::EmitData(AsmPrinter *Asm, DwarfFile *D,<br>+void DwarfAccelTable::EmitData(AsmPrinter *Asm, DwarfDebug *D,<br> <span> </span>MCSymbol *StrSym) {<br> uint64_t PrevHash = UINT64_MAX;<br> for (size_t i = 0, e = Buckets.size(); i < e; ++i) {<br>@@ -189,7 +190,9 @@ void DwarfAccelTable::EmitData(AsmPrinte<br> Asm->EmitInt32((*HI)->Data.Values.size());<br> for (HashDataContents *HD : (*HI)->Data.Values) {<br> // Emit the DIE offset<br>- Asm->EmitInt32(HD->Die->getOffset());<br>+ DwarfCompileUnit *CU = D->lookupUnit(HD->Die->getUnit());<br></blockquote><div><br></div><div>'getUnit()' isn't the cheapest operation (having to walk the DIE tree checking for the top level unit kind) if it can be avoided (& then the extra map lookup too) - just FYI in case there's a nicer way to do this (should we record the DWARF*Unit in the record when we insert it instead?)<br><br>No big deal - just a thought.</div></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>Mmmmh. Storing the Unit when we register the accelerator is a good idea. I thought about adding a Unit field to the DIE when writing this fix exactly to avoid the getUnit() lookup, but knowing that the DIEs account for a lot of memory already I didn’t do it. Paying the price in the AccelTable object it self should be fine though.</div><div><br></div><div>Fred</div></div></div><div style="word-wrap:break-word"><div><div><br></div><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">+ assert(CU && "Accelerated DIE should belong to a CU.");<br>+ Asm->EmitInt32(HD->Die->getOffset() + CU->getDebugInfoOffset());<br> // If we have multiple Atoms emit that info too.<br> // FIXME: A bit of a hack, we either emit only one atom or all info.<br> if (HeaderData.Atoms.size() > 1) {<br>@@ -206,7 +209,7 @@ void DwarfAccelTable::EmitData(AsmPrinte<br> }<br><br> // Emit the entire data structure to the output file.<br>-void DwarfAccelTable::Emit(AsmPrinter *Asm, MCSymbol *SecBegin, DwarfFile *D,<br>+void DwarfAccelTable::Emit(AsmPrinter *Asm, MCSymbol *SecBegin, DwarfDebug *D,<br> <span> </span>MCSymbol *StrSym) {<br> // Emit the header.<br> EmitHeader(Asm);<br><br>Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h?rev=221837&r1=221836&r2=221837&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h?rev=221837&r1=221836&r2=221837&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h (original)<br>+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h Wed Nov 12 17:48:14 2014<br>@@ -62,7 +62,7 @@<br> namespace llvm {<br><br> class AsmPrinter;<br>-class DwarfFile;<br>+class DwarfDebug;<br><br> class DwarfAccelTable {<br><br>@@ -223,7 +223,7 @@ private:<br> void EmitBuckets(AsmPrinter *);<br> void EmitHashes(AsmPrinter *);<br> void EmitOffsets(AsmPrinter *, MCSymbol *);<br>- void EmitData(AsmPrinter *, DwarfFile *D, MCSymbol *StrSym);<br>+ void EmitData(AsmPrinter *, DwarfDebug *D, MCSymbol *StrSym);<br><br> // Allocator for HashData and HashDataContents.<br> BumpPtrAllocator Allocator;<br>@@ -248,7 +248,7 @@ public:<br> void AddName(StringRef Name, MCSymbol *StrSym, const DIE *Die,<br> <span> </span>char Flags = 0);<br> void FinalizeTable(AsmPrinter *, StringRef);<br>- void Emit(AsmPrinter *, MCSymbol *, DwarfFile *, MCSymbol *StrSym);<br>+ void Emit(AsmPrinter *, MCSymbol *, DwarfDebug *, MCSymbol *StrSym);<br> #ifndef NDEBUG<br> void print(raw_ostream &O);<br> void dump() { print(dbgs()); }<br><br>Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=221837&r1=221836&r2=221837&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=221837&r1=221836&r2=221837&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>+++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed Nov 12 17:48:14 2014<br>@@ -1478,7 +1478,7 @@ void DwarfDebug::emitAccel(DwarfAccelTab<br> Asm->OutStreamer.EmitLabel(SectionBegin);<br><br> // Emit the full data.<br>- Accel.Emit(Asm, SectionBegin, &InfoHolder, DwarfStrSectionSym);<br>+ Accel.Emit(Asm, SectionBegin, this, DwarfStrSectionSym);<br> }<br><br> // Emit visible names into a hashed accelerator table section.<br><br>Modified: llvm/trunk/test/DebugInfo/cross-cu-inlining.ll<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/cross-cu-inlining.ll?rev=221837&r1=221836&r2=221837&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/cross-cu-inlining.ll?rev=221837&r1=221836&r2=221837&view=diff</a><br>==============================================================================<br>--- llvm/trunk/test/DebugInfo/cross-cu-inlining.ll (original)<br>+++ llvm/trunk/test/DebugInfo/cross-cu-inlining.ll Wed Nov 12 17:48:14 2014<br>@@ -1,6 +1,7 @@<br> ; REQUIRES: object-emission<br><br> ; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump -debug-dump=info - | FileCheck -implicit-check-not=DW_TAG %s<br>+; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump - | FileCheck --check-prefix=CHECK-ACCEL --check-prefix=CHECK %s<br><br> ; Build from source:<br> ; $ clang++ a.cpp b.cpp -g -c -emit-llvm<br>@@ -24,7 +25,7 @@<br> ; CHECK: DW_AT_name {{.*}} "a.cpp"<br> ; CHECK: DW_TAG_subprogram<br> ; CHECK: DW_AT_type [DW_FORM_ref_addr] (0x00000000[[INT:.*]])<br>-; CHECK: DW_TAG_inlined_subroutine<br>+; CHECK: 0x[[INLINED:[0-9a-f]*]]:{{.*}}DW_TAG_inlined_subroutine<br> ; CHECK: DW_AT_abstract_origin {{.*}}[[ABS_FUNC:........]] "_Z4funci"<br> ; CHECK: DW_TAG_formal_parameter<br> ; CHECK: DW_AT_abstract_origin {{.*}}[[ABS_VAR:........]] "x"<br>@@ -45,13 +46,24 @@<br><br> ; Check the concrete out of line definition references the abstract and<br> ; provides the address range and variable location<br>-; CHECK: DW_TAG_subprogram<br>+; CHECK: 0x[[FUNC:[0-9a-f]*]]{{.*}}DW_TAG_subprogram<br></blockquote><div><br>I tend to prefer to include the 0x in the match, just to make sure the later backreference doesn't have any other junk preceeding it, etc.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> ; CHECK: DW_AT_low_pc<br> ; CHECK: DW_AT_abstract_origin {{.*}} {0x[[ABS_FUNC]]} "_Z4funci"<br> ; CHECK: DW_TAG_formal_parameter<br> ; CHECK: DW_AT_location<br> ; CHECK: DW_AT_abstract_origin {{.*}} {0x[[ABS_VAR]]} "x"<br><br>+; CHECK-ACCEL: .apple_names contents:<br>+; CHECK-ACCEL: Name{{.*}}"func"<br>+; CHECK-ACCEL-NOT: Name<br>+; CHECK-ACCEL: Atom[0]{{.*}}[[INLINED]]<br>+; CHECK-ACCEL-NOT: Name<br>+; CHECK-ACCEL: Atom[0]{{.*}}[[FUNC]]<br>+<br>+; CHECK-ACCEL: .apple_types contents:<br>+; CHECK-ACCEL: Name{{.*}}"int"<br>+; CHECK-ACCEL-NOT: Name<br>+; CHECK-ACCEL: Atom[0]{{.*}}[[INT]]<br></blockquote><div><br>I'm assuming not all 3 of these elements were in the non-first CU, so some of these tests could've been added separately? (ie: they were already passing before your fix in this commit)<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br> @i = external global i32<br><br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div></div></blockquote></div></div>______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</blockquote></div>