[llvm] r221837 - Fix emission of Dwarf accelerator table when there are multiple CUs.
Eric Christopher
echristo at gmail.com
Wed Nov 12 17:38:07 PST 2014
Agreed, it'll be fine.
-eric
On Wed Nov 12 2014 at 5:32:41 PM Frédéric Riss <friss at apple.com> wrote:
> On Nov 12, 2014, at 4:40 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Wed, Nov 12, 2014 at 3:48 PM, Frederic Riss <friss at apple.com> wrote:
>
>> Author: friss
>> Date: Wed Nov 12 17:48:14 2014
>> New Revision: 221837
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=221837&view=rev
>> Log:
>> Fix emission of Dwarf accelerator table when there are multiple CUs.
>>
>> The DIE offset in the accel tables is an offset relative to the start
>> of the debug_info section, but we were encoding the offset to the
>> start of the containing CU.
>>
>> Modified:
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> llvm/trunk/test/DebugInfo/cross-cu-inlining.ll
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp?rev=221837&r1=221836&r2=221837&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.cpp Wed Nov 12
>> 17:48:14 2014
>> @@ -13,6 +13,7 @@
>>
>> #include "DwarfAccelTable.h"
>> #include "DIE.h"
>> +#include "DwarfCompileUnit.h"
>> #include "DwarfDebug.h"
>> #include "llvm/ADT/STLExtras.h"
>> #include "llvm/ADT/Twine.h"
>> @@ -174,7 +175,7 @@ void DwarfAccelTable::EmitOffsets(AsmPri
>> // Walk through the buckets and emit the full data for each element in
>> // the bucket. For the string case emit the dies and the various offsets.
>> // Terminate each HashData bucket with 0.
>> -void DwarfAccelTable::EmitData(AsmPrinter *Asm, DwarfFile *D,
>> +void DwarfAccelTable::EmitData(AsmPrinter *Asm, DwarfDebug *D,
>> MCSymbol *StrSym) {
>> uint64_t PrevHash = UINT64_MAX;
>> for (size_t i = 0, e = Buckets.size(); i < e; ++i) {
>> @@ -189,7 +190,9 @@ void DwarfAccelTable::EmitData(AsmPrinte
>> Asm->EmitInt32((*HI)->Data.Values.size());
>> for (HashDataContents *HD : (*HI)->Data.Values) {
>> // Emit the DIE offset
>> - Asm->EmitInt32(HD->Die->getOffset());
>> + DwarfCompileUnit *CU = D->lookupUnit(HD->Die->getUnit());
>>
>
> '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?)
>
> No big deal - just a thought.
>
>
> 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.
>
> Fred
>
> + assert(CU && "Accelerated DIE should belong to a CU.");
>> + Asm->EmitInt32(HD->Die->getOffset() + CU->getDebugInfoOffset());
>> // If we have multiple Atoms emit that info too.
>> // FIXME: A bit of a hack, we either emit only one atom or all
>> info.
>> if (HeaderData.Atoms.size() > 1) {
>> @@ -206,7 +209,7 @@ void DwarfAccelTable::EmitData(AsmPrinte
>> }
>>
>> // Emit the entire data structure to the output file.
>> -void DwarfAccelTable::Emit(AsmPrinter *Asm, MCSymbol *SecBegin,
>> DwarfFile *D,
>> +void DwarfAccelTable::Emit(AsmPrinter *Asm, MCSymbol *SecBegin,
>> DwarfDebug *D,
>> MCSymbol *StrSym) {
>> // Emit the header.
>> EmitHeader(Asm);
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h?rev=221837&r1=221836&r2=221837&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfAccelTable.h Wed Nov 12
>> 17:48:14 2014
>> @@ -62,7 +62,7 @@
>> namespace llvm {
>>
>> class AsmPrinter;
>> -class DwarfFile;
>> +class DwarfDebug;
>>
>> class DwarfAccelTable {
>>
>> @@ -223,7 +223,7 @@ private:
>> void EmitBuckets(AsmPrinter *);
>> void EmitHashes(AsmPrinter *);
>> void EmitOffsets(AsmPrinter *, MCSymbol *);
>> - void EmitData(AsmPrinter *, DwarfFile *D, MCSymbol *StrSym);
>> + void EmitData(AsmPrinter *, DwarfDebug *D, MCSymbol *StrSym);
>>
>> // Allocator for HashData and HashDataContents.
>> BumpPtrAllocator Allocator;
>> @@ -248,7 +248,7 @@ public:
>> void AddName(StringRef Name, MCSymbol *StrSym, const DIE *Die,
>> char Flags = 0);
>> void FinalizeTable(AsmPrinter *, StringRef);
>> - void Emit(AsmPrinter *, MCSymbol *, DwarfFile *, MCSymbol *StrSym);
>> + void Emit(AsmPrinter *, MCSymbol *, DwarfDebug *, MCSymbol *StrSym);
>> #ifndef NDEBUG
>> void print(raw_ostream &O);
>> void dump() { print(dbgs()); }
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=221837&r1=221836&r2=221837&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed Nov 12 17:48:14
>> 2014
>> @@ -1478,7 +1478,7 @@ void DwarfDebug::emitAccel(DwarfAccelTab
>> Asm->OutStreamer.EmitLabel(SectionBegin);
>>
>> // Emit the full data.
>> - Accel.Emit(Asm, SectionBegin, &InfoHolder, DwarfStrSectionSym);
>> + Accel.Emit(Asm, SectionBegin, this, DwarfStrSectionSym);
>> }
>>
>> // Emit visible names into a hashed accelerator table section.
>>
>> Modified: llvm/trunk/test/DebugInfo/cross-cu-inlining.ll
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/cross-cu-inlining.ll?rev=221837&r1=221836&r2=221837&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/test/DebugInfo/cross-cu-inlining.ll (original)
>> +++ llvm/trunk/test/DebugInfo/cross-cu-inlining.ll Wed Nov 12 17:48:14
>> 2014
>> @@ -1,6 +1,7 @@
>> ; REQUIRES: object-emission
>>
>> ; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump
>> -debug-dump=info - | FileCheck -implicit-check-not=DW_TAG %s
>> +; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump - | FileCheck
>> --check-prefix=CHECK-ACCEL --check-prefix=CHECK %s
>>
>> ; Build from source:
>> ; $ clang++ a.cpp b.cpp -g -c -emit-llvm
>> @@ -24,7 +25,7 @@
>> ; CHECK: DW_AT_name {{.*}} "a.cpp"
>> ; CHECK: DW_TAG_subprogram
>> ; CHECK: DW_AT_type [DW_FORM_ref_addr] (0x00000000[[INT:.*]])
>> -; CHECK: DW_TAG_inlined_subroutine
>> +; CHECK: 0x[[INLINED:[0-9a-f]*]]:{{.*}}DW_TAG_inlined_subroutine
>> ; CHECK: DW_AT_abstract_origin {{.*}}[[ABS_FUNC:........]]
>> "_Z4funci"
>> ; CHECK: DW_TAG_formal_parameter
>> ; CHECK: DW_AT_abstract_origin {{.*}}[[ABS_VAR:........]] "x"
>> @@ -45,13 +46,24 @@
>>
>> ; Check the concrete out of line definition references the abstract and
>> ; provides the address range and variable location
>> -; CHECK: DW_TAG_subprogram
>> +; CHECK: 0x[[FUNC:[0-9a-f]*]]{{.*}}DW_TAG_subprogram
>>
>
> 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.
>
>
>> ; CHECK: DW_AT_low_pc
>> ; CHECK: DW_AT_abstract_origin {{.*}} {0x[[ABS_FUNC]]} "_Z4funci"
>> ; CHECK: DW_TAG_formal_parameter
>> ; CHECK: DW_AT_location
>> ; CHECK: DW_AT_abstract_origin {{.*}} {0x[[ABS_VAR]]} "x"
>>
>> +; CHECK-ACCEL: .apple_names contents:
>> +; CHECK-ACCEL: Name{{.*}}"func"
>> +; CHECK-ACCEL-NOT: Name
>> +; CHECK-ACCEL: Atom[0]{{.*}}[[INLINED]]
>> +; CHECK-ACCEL-NOT: Name
>> +; CHECK-ACCEL: Atom[0]{{.*}}[[FUNC]]
>> +
>> +; CHECK-ACCEL: .apple_types contents:
>> +; CHECK-ACCEL: Name{{.*}}"int"
>> +; CHECK-ACCEL-NOT: Name
>> +; CHECK-ACCEL: Atom[0]{{.*}}[[INT]]
>>
>
> 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)
>
>
>>
>> @i = external global i32
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141113/97dc759e/attachment.html>
More information about the llvm-commits
mailing list