[llvm] r198196 - Use a pointer to keep track of the skeleton unit for each normal unit

dblaikie at gmail.com dblaikie at gmail.com
Mon Dec 30 08:38:19 PST 2013


On Sun Dec 29 2013 at 7:08:41 PM, Eric Christopher <echristo at gmail.com>
wrote:

> Author: echristo
> Date: Sun Dec 29 21:02:12 2013
> New Revision: 198196
>
> URL: http://llvm.org/viewvc/llvm-project?rev=198196&view=rev
> Log:
> Use a pointer to keep track of the skeleton unit for each normal unit
> and construct it up front. Add address ranges at the end and a helper
> routine so that we're not needlessly using an indirction in the case
> of split dwarf.
>
> Update testcases according to the new ordering of attributes on
> the compile unit.
>
> Modified:
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
>     llvm/trunk/test/DebugInfo/X86/fission-cu.ll
>     llvm/trunk/test/DebugInfo/X86/stmt-list-multiple-compile-units.ll
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> CodeGen/AsmPrinter/DwarfDebug.cpp?rev=198196&r1=198195&r2=198196&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Sun Dec 29 21:02:12
> 2013
> @@ -773,12 +773,6 @@ DwarfCompileUnit *DwarfDebug::constructD
>                   DIUnit.getLanguage());
>    NewCU->addString(Die, dwarf::DW_AT_name, FN);
>
> -  // 2.17.1 requires that we use DW_AT_low_pc for a single entry point
> -  // into an entity. We're using 0 (or a NULL label) for this. For
> -  // split dwarf it's in the skeleton CU so omit it here.
> -  if (!useSplitDwarf())
> -    NewCU->addLabelAddress(Die, dwarf::DW_AT_low_pc, NULL);
> -
>    // Define start line table label for each Compile Unit.
>    MCSymbol *LineTableStartSym =
>        Asm->GetTempSymbol("line_table_start", NewCU->getUniqueID());
> @@ -837,6 +831,10 @@ DwarfCompileUnit *DwarfDebug::constructD
>        // skeleton units, not full units, if it's going to reference
> skeletons
>        DwarfInfoSectionSym);
>
> +  // If we're splitting the dwarf then construct the skeleton CU now.
> +  if (useSplitDwarf())
> +    NewCU->setSkeleton(constructSkeletonCU(NewCU));
> +
>    CUMap.insert(std::make_pair(DIUnit, NewCU));
>    CUDieMap.insert(std::make_pair(Die, NewCU));
>    return NewCU;
> @@ -1082,7 +1080,9 @@ void DwarfDebug::finalizeModuleInfo() {
>      // Add CU specific attributes if we need to add any.
>      if (TheU->getUnitDie()->getTag() == dwarf::DW_TAG_compile_unit) {
>        // If we're splitting the dwarf out now that we've got the entire
> -      // CU then construct a skeleton CU based upon it.
> +      // CU then add the dwo id to it.
> +      DwarfCompileUnit *SkCU =
> +          static_cast<DwarfCompileUnit *>(TheU->getSkeleton());
>

Potentially we could change the check above (== compile_unit) to use
dyn_cast on the DwarfUnit itself (would need to add cast machinery to
DwarfUnit, I suppose) and add covariant overrides to getSkeleton in
Dwarf*Unit so that DwarfCompileUnit::getSkeleton returns a DwarfCompileUnit
& you wouldn't need the cast here.

Actually - do you need anything from DwarfCompileUnit specifically here? Or
could you just make "SkCU" a DwarfUnit? (self documenting code is a fine
reason, just not sure if it's the only one here)



>        if (useSplitDwarf()) {
>          // This should be a unique identifier when we want to build .dwp
> files.
>          uint64_t ID = 0;
> @@ -1092,19 +1092,22 @@ void DwarfDebug::finalizeModuleInfo() {
>          }
>          TheU->addUInt(TheU->getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
>                        dwarf::DW_FORM_data8, ID);
> -        // Now construct the skeleton CU associated.
> -        DwarfCompileUnit *SkCU =
> -            constructSkeletonCU(static_cast<DwarfCompileUnit *>(TheU));
>          SkCU->addUInt(SkCU->getUnitDie(), dwarf::DW_AT_GNU_dwo_id,
>                        dwarf::DW_FORM_data8, ID);
> -      } else {
> -        // Attribute if we've emitted a range list for the compile unit,
> this
> -        // will get constructed for the skeleton CU separately if we have
> one.
> -        if (DwarfCURanges && TheU->getRanges().size())
> -          addSectionLabel(Asm, TheU, TheU->getUnitDie(),
> dwarf::DW_AT_ranges,
> -                          Asm->GetTempSymbol("cu_ranges",
> TheU->getUniqueID()),
> -                          DwarfDebugRangeSectionSym);
>        }
> +
> +      // If we've requested ranges and have them emit a DW_AT_ranges
> attribute
> +      // on the unit that will remain in the .o file, otherwise add a
> DW_AT_low_pc.
> +      // FIXME: Also add a high pc if we can.
> +      // FIXME: We should use ranges if we have multiple compile units.
> +      DwarfCompileUnit *U = SkCU ? SkCU : static_cast<DwarfCompileUnit
> *>(TheU);
>

You could just make "U" here a DwarfUnit, since you don't need any
DwarfCompileUnit-specific behavior below. Or if you did the cast stuff
suggested above, TheU would be a DwarfCompileUnit already.

(could do this half-way, too. Without adding cast machinery to DwarfUnit,
you could keep the == compile_unit check, then create a local variable with
the static_cast of TheU, then use that both at the first comment (if you
added the covariant return overrides) and here)


> +      if (DwarfCURanges && TheU->getRanges().size())
> +        addSectionLabel(Asm, U, U->getUnitDie(), dwarf::DW_AT_ranges,
> +                        Asm->GetTempSymbol("cu_ranges",
> U->getUniqueID()),
> +                        DwarfDebugRangeSectionSym);
> +      else
> +        U->addLocalLabelAddress(U->getUnitDie(), dwarf::DW_AT_low_pc,
> +                                TextSectionSym);
>      }
>    }
>
> @@ -2997,15 +3000,6 @@ DwarfCompileUnit *DwarfDebug::constructS
>    else
>      NewCU->addSectionOffset(Die, dwarf::DW_AT_GNU_addr_base, 0);
>
> -  // Attribute if we've emitted a range list for the compile unit, this
> -  // will get constructed for the skeleton CU separately if we have one.
> -  if (DwarfCURanges && CU->getRanges().size())
> -    addSectionLabel(Asm, NewCU, Die, dwarf::DW_AT_ranges,
> -                    Asm->GetTempSymbol("cu_ranges", CU->getUniqueID()),
> -                    DwarfDebugRangeSectionSym);
> -  else
> -    NewCU->addUInt(Die, dwarf::DW_AT_low_pc, dwarf::DW_FORM_addr, 0);
> -
>    // DW_AT_stmt_list is a offset of line number information for this
>    // compile unit in debug_line section.
>    // FIXME: Should handle multiple compile units.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> CodeGen/AsmPrinter/DwarfUnit.cpp?rev=198196&r1=198195&r2=198196&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Sun Dec 29 21:02:12
> 2013
> @@ -293,6 +293,23 @@ void DwarfCompileUnit::addLabelAddress(D
>    }
>  }
>
> +/// addLocalLabelAddress - Add a dwarf label attribute data and value
> using
> +/// DW_FORM_addr.
> +void DwarfCompileUnit::addLocalLabelAddress(DIE *Die,
> +                                            dwarf::Attribute Attribute,
> +                                            MCSymbol *Label) {
>

Is this refactoring out some common code? I don't see this function being
removed from somewhere else. Might've been easier to see the refactor if it
were done separately.


> +  if (Label)
> +    DD->addArangeLabel(SymbolCU(this, Label));
> +
> +  if (Label != NULL) {
>

The same if condition twice? Written in two different ways?


> +    DIEValue *Value = new (DIEValueAllocator) DIELabel(Label);
> +    Die->addValue(Attribute, dwarf::DW_FORM_addr, Value);
> +  } else {
> +    DIEValue *Value = new (DIEValueAllocator) DIEInteger(0);
> +    Die->addValue(Attribute, dwarf::DW_FORM_addr, Value);
> +  }
> +}
> +
>  /// addOpAddress - Add a dwarf op address data and value using the
>  /// form given and an op of either DW_FORM_addr or DW_FORM_GNU_addr_index.
>  ///
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/
> CodeGen/AsmPrinter/DwarfUnit.h?rev=198196&r1=198195&r2=198196&view=diff
> ============================================================
> ==================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h Sun Dec 29 21:02:12 2013
> @@ -146,12 +146,21 @@ protected:
>    /// The label for the start of the range sets for the elements of this
> unit.
>    MCSymbol *LabelRange;
>
> +  /// Skeleton unit associated with this unit.
> +  DwarfUnit *Skeleton;
> +
>    DwarfUnit(unsigned UID, DIE *D, DICompileUnit CU, AsmPrinter *A,
>              DwarfDebug *DW, DwarfFile *DWU);
>
>  public:
>    virtual ~DwarfUnit();
>
> +  /// Set the skeleton unit associated with this unit.
> +  void setSkeleton(DwarfUnit *Skel) { Skeleton = Skel; }
> +
> +  /// Get the skeleton unit associated with this unit.
> +  DwarfUnit *getSkeleton() const { return Skeleton; }
> +
>    /// Pass in the SectionSym even though we could recreate it in every
> compile
>    /// unit (type units will have actually distinct symbols once they're in
>    /// comdat sections).
> @@ -521,6 +530,11 @@ public:
>    /// either DW_FORM_addr or DW_FORM_GNU_addr_index.
>    void addLabelAddress(DIE *Die, dwarf::Attribute Attribute, MCSymbol
> *Label);
>
> +  /// addLocalLabelAddress - Add a dwarf label attribute data and value
> using
> +  /// DW_FORM_addr.
> +  void addLocalLabelAddress(DIE *Die, dwarf::Attribute Attribute,
> +                            MCSymbol *Label);
> +
>    uint16_t getLanguage() const LLVM_OVERRIDE { return
> getNode().getLanguage(); }
>  };
>
>
> Modified: llvm/trunk/test/DebugInfo/X86/fission-cu.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> DebugInfo/X86/fission-cu.ll?rev=198196&r1=198195&r2=198196&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/DebugInfo/X86/fission-cu.ll (original)
> +++ llvm/trunk/test/DebugInfo/X86/fission-cu.ll Sun Dec 29 21:02:12 2013
> @@ -25,19 +25,19 @@
>  ; CHECK: [1] DW_TAG_compile_unit DW_CHILDREN_no
>  ; CHECK: DW_AT_GNU_dwo_name      DW_FORM_strp
>  ; CHECK: DW_AT_GNU_addr_base     DW_FORM_sec_offset
> -; CHECK: DW_AT_low_pc    DW_FORM_addr
>  ; CHECK: DW_AT_stmt_list DW_FORM_sec_offset
>  ; CHECK: DW_AT_comp_dir  DW_FORM_strp
>  ; CHECK: DW_AT_GNU_dwo_id        DW_FORM_data8
> +; CHECK: DW_AT_low_pc    DW_FORM_addr
>
>  ; CHECK: .debug_info contents:
>  ; CHECK: DW_TAG_compile_unit
>  ; CHECK: DW_AT_GNU_dwo_name [DW_FORM_strp] ( .debug_str[0x00000000] =
> "baz.dwo")
>  ; CHECK: DW_AT_GNU_addr_base [DW_FORM_sec_offset]
> (0x00000000)
> -; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>  ; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x00000000)
>  ; CHECK: DW_AT_comp_dir [DW_FORM_strp]     ( .debug_str[0x00000008] =
> "/usr/local/google/home/echristo/tmp")
>  ; CHECK: DW_AT_GNU_dwo_id [DW_FORM_data8]  (0x0000000000000000)
> +; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>
>  ; CHECK: .debug_str contents:
>  ; CHECK: 0x00000000: "baz.dwo"
> @@ -110,5 +110,6 @@
>  ; OBJ-NEXT: R_X86_64_32 .debug_addr
>  ; OBJ-NEXT: R_X86_64_32 .debug_line
>  ; OBJ-NEXT: R_X86_64_32 .debug_str
> +; OBJ-NEXT: R_X86_64_64 .text 0x0
>  ; OBJ-NEXT: }
>  !9 = metadata !{i32 1, metadata !"Debug Info Version", i32 1}
>
> Modified: llvm/trunk/test/DebugInfo/X86/stmt-list-multiple-compile-
> units.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/
> DebugInfo/X86/stmt-list-multiple-compile-units.ll?rev=
> 198196&r1=198195&r2=198196&view=diff
> ============================================================
> ==================
> --- llvm/trunk/test/DebugInfo/X86/stmt-list-multiple-compile-units.ll
> (original)
> +++ llvm/trunk/test/DebugInfo/X86/stmt-list-multiple-compile-units.ll Sun
> Dec 29 21:02:12 2013
> @@ -7,12 +7,12 @@
>  ; rdar://13067005
>  ; CHECK: .debug_info contents:
>  ; CHECK: DW_TAG_compile_unit
> -; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>  ; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x00000000)
> +; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>
>  ; CHECK: DW_TAG_compile_unit
> -; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>  ; CHECK: DW_AT_stmt_list [DW_FORM_sec_offset]   (0x0000003c)
> +; CHECK: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>
>  ; CHECK: .debug_line contents:
>  ; CHECK-NEXT: Line table prologue:
> @@ -25,12 +25,12 @@
>
>  ; DWARF3: .debug_info contents:
>  ; DWARF3: DW_TAG_compile_unit
> -; DWARF3: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>  ; DWARF3: DW_AT_stmt_list [DW_FORM_data4]   (0x00000000)
> +; DWARF3: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>
>  ; DWARF3: DW_TAG_compile_unit
> -; DWARF3: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>  ; DWARF3: DW_AT_stmt_list [DW_FORM_data4]   (0x0000003c)
> +; DWARF3: DW_AT_low_pc [DW_FORM_addr]       (0x0000000000000000)
>
>  ; DWARF3: .debug_line contents:
>  ; DWARF3-NEXT: Line table prologue:
>
>
> _______________________________________________
> 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/20131230/778b75b4/attachment.html>


More information about the llvm-commits mailing list