[llvm] r221837 - Fix emission of Dwarf accelerator table when there are multiple CUs.

Frédéric Riss friss at apple.com
Wed Nov 12 17:29:55 PST 2014


> 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 <mailto: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 <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 <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 <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 <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 <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 <mailto:llvm-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits <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/20141112/7bd2bc2a/attachment.html>


More information about the llvm-commits mailing list