[llvm] r196178 - Make ranges and range lists be a discrete entity that can be located

David Blaikie dblaikie at gmail.com
Mon Dec 2 20:33:40 PST 2013


On Mon, Dec 2, 2013 at 4:45 PM, Eric Christopher <echristo at gmail.com> wrote:
> Author: echristo
> Date: Mon Dec  2 18:45:45 2013
> New Revision: 196178
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196178&view=rev
> Log:
> Make ranges and range lists be a discrete entity that can be located
> and emitted per function and CU. Begins coalescing ranges as a first
> class entity through debug info. No functional change.
>
> Modified:
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=196178&r1=196177&r2=196178&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Dec  2 18:45:45 2013
> @@ -185,8 +185,8 @@ DwarfDebug::DwarfDebug(AsmPrinter *A, Mo
>      : Asm(A), MMI(Asm->MMI), FirstCU(0),
>        AbbreviationsSet(InitAbbreviationsSetSize),
>        SourceIdMap(DIEValueAllocator), PrevLabel(NULL), GlobalCUIndexCount(0),
> -      InfoHolder(A, &AbbreviationsSet, Abbreviations, "info_string",
> -                 DIEValueAllocator),
> +      GlobalRangeCount(0), InfoHolder(A, &AbbreviationsSet, Abbreviations,
> +                                      "info_string", DIEValueAllocator),
>        SkeletonAbbrevSet(InitAbbreviationsSetSize),
>        SkeletonHolder(A, &SkeletonAbbrevSet, SkeletonAbbrevs, "skel_string",
>                       DIEValueAllocator) {
> @@ -429,12 +429,12 @@ DIE *DwarfDebug::updateSubprogramScopeDI
>      }
>    }
>
> -  SPCU->addLabelAddress(
> -      SPDie, dwarf::DW_AT_low_pc,
> -      Asm->GetTempSymbol("func_begin", Asm->getFunctionNumber()));
> -  SPCU->addLabelAddress(
> -      SPDie, dwarf::DW_AT_high_pc,
> -      Asm->GetTempSymbol("func_end", Asm->getFunctionNumber()));
> +  MCSymbol *FuncBegin =
> +      Asm->GetTempSymbol("func_begin", Asm->getFunctionNumber());
> +  MCSymbol *FuncEnd = Asm->GetTempSymbol("func_end", Asm->getFunctionNumber());
> +  SPCU->addLabelAddress(SPDie, dwarf::DW_AT_low_pc, FuncBegin);
> +  SPCU->addLabelAddress(SPDie, dwarf::DW_AT_high_pc, FuncEnd);
> +
>    const TargetRegisterInfo *RI = Asm->TM.getRegisterInfo();
>    MachineLocation Location(RI->getFrameRegister(*Asm->MF));
>    SPCU->addAddress(SPDie, dwarf::DW_AT_frame_base, Location);
> @@ -478,30 +478,31 @@ DIE *DwarfDebug::constructLexicalScopeDI
>    if (Scope->isAbstractScope())
>      return ScopeDIE;
>
> -  const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();
> +  const SmallVectorImpl<InsnRange> &ScopeRanges = Scope->getRanges();
>    // If we have multiple ranges, emit them into the range section.
> -  if (Ranges.size() > 1) {
> +  if (ScopeRanges.size() > 1) {
>      // .debug_range section has not been laid out yet. Emit offset in
>      // .debug_range as a relocatable label. emitDIE will handle
>      // emitting it appropriately.
> -    unsigned Offset = DebugRangeSymbols.size();
> -    TheCU->addSectionLabel(ScopeDIE, dwarf::DW_AT_ranges,
> -                           Asm->GetTempSymbol("debug_ranges", Offset));
> -    for (SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin(),
> -                                                    RE = Ranges.end();
> +    TheCU->addSectionLabel(
> +        ScopeDIE, dwarf::DW_AT_ranges,
> +        Asm->GetTempSymbol("debug_ranges", GlobalRangeCount));
> +    RangeSpanList *List = new RangeSpanList(GlobalRangeCount++);

Would it be reasonable to just stack-allocate RangeSpanLists?

> +    for (SmallVectorImpl<InsnRange>::const_iterator RI = ScopeRanges.begin(),
> +                                                    RE = ScopeRanges.end();
>           RI != RE; ++RI) {
> -      DebugRangeSymbols.push_back(getLabelBeforeInsn(RI->first));
> -      DebugRangeSymbols.push_back(getLabelAfterInsn(RI->second));
> +      RangeSpan Range(getLabelBeforeInsn(RI->first),
> +                      getLabelAfterInsn(RI->second));
> +      List->addRange(Range);
>      }
>
> -    // Terminate the range list.
> -    DebugRangeSymbols.push_back(NULL);
> -    DebugRangeSymbols.push_back(NULL);
> +    // Add the range list to the set of ranges to be emitted.
> +    TheCU->addRangeList(List);

You could use llvm_move when passing the list to addRangeList here.

>      return ScopeDIE;
>    }
>
>    // Construct the address range for this DIE.
> -  SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();
> +  SmallVectorImpl<InsnRange>::const_iterator RI = ScopeRanges.begin();
>    MCSymbol *Start = getLabelBeforeInsn(RI->first);
>    MCSymbol *End = getLabelAfterInsn(RI->second);
>    assert(End && "End label should not be null!");
> @@ -519,8 +520,8 @@ DIE *DwarfDebug::constructLexicalScopeDI
>  // represent this concrete inlined copy of the function.
>  DIE *DwarfDebug::constructInlinedScopeDIE(CompileUnit *TheCU,
>                                            LexicalScope *Scope) {
> -  const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();
> -  assert(Ranges.empty() == false &&
> +  const SmallVectorImpl<InsnRange> &ScopeRanges = Scope->getRanges();
> +  assert(ScopeRanges.empty() == false &&

Might be nice to change x == false to !x.

>           "LexicalScope does not have instruction markers!");
>
>    if (!Scope->getScopeNode())
> @@ -536,23 +537,26 @@ DIE *DwarfDebug::constructInlinedScopeDI
>    DIE *ScopeDIE = new DIE(dwarf::DW_TAG_inlined_subroutine);
>    TheCU->addDIEEntry(ScopeDIE, dwarf::DW_AT_abstract_origin, OriginDIE);
>
> -  if (Ranges.size() > 1) {
> +  if (ScopeRanges.size() > 1) {
>      // .debug_range section has not been laid out yet. Emit offset in
>      // .debug_range as a relocatable label. emitDIE will handle
>      // emitting it appropriately.
> -    unsigned Offset = DebugRangeSymbols.size();
> -    TheCU->addSectionLabel(ScopeDIE, dwarf::DW_AT_ranges,
> -                           Asm->GetTempSymbol("debug_ranges", Offset));
> -    for (SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin(),
> -                                                    RE = Ranges.end();
> +    TheCU->addSectionLabel(
> +        ScopeDIE, dwarf::DW_AT_ranges,
> +        Asm->GetTempSymbol("debug_ranges", GlobalRangeCount));
> +    RangeSpanList *List = new RangeSpanList(GlobalRangeCount++);

Might be useful to extract this logic and the above matching code into
one function (perhaps CompileUnit should just have a function for
adding a new RangeSpanList and returning a non-const reference to it?)

> +    for (SmallVectorImpl<InsnRange>::const_iterator RI = ScopeRanges.begin(),
> +                                                    RE = ScopeRanges.end();
>           RI != RE; ++RI) {
> -      DebugRangeSymbols.push_back(getLabelBeforeInsn(RI->first));
> -      DebugRangeSymbols.push_back(getLabelAfterInsn(RI->second));
> +      RangeSpan Range(getLabelBeforeInsn(RI->first),
> +                      getLabelAfterInsn(RI->second));
> +      List->addRange(Range);
>      }

This loop looks the same as the one above - any reason it can't all be
deduplicated?

> -    DebugRangeSymbols.push_back(NULL);
> -    DebugRangeSymbols.push_back(NULL);
> +
> +    // Add the range list to the set of ranges to be emitted.
> +    TheCU->addRangeList(List);
>    } else {
> -    SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();
> +    SmallVectorImpl<InsnRange>::const_iterator RI = ScopeRanges.begin();
>      MCSymbol *StartLabel = getLabelBeforeInsn(RI->first);
>      MCSymbol *EndLabel = getLabelAfterInsn(RI->second);
>
> @@ -2922,18 +2926,51 @@ void DwarfDebug::emitDebugRanges() {
>    // Start the dwarf ranges section.
>    Asm->OutStreamer.SwitchSection(
>        Asm->getObjFileLowering().getDwarfRangesSection());
> +
> +  // Size for our labels.
>    unsigned char Size = Asm->getDataLayout().getPointerSize();
> -  for (uint32_t i = 0, e = DebugRangeSymbols.size(); i < e; ++i) {
> -    // Only emit a symbol for every range pair for now.
> -    // FIXME: Make this per range list.
> -    if ((i % 2) == 0)
> -      Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("debug_ranges", i));
> -
> -    const MCSymbol *I = DebugRangeSymbols[i];
> -    if (I)
> -      Asm->OutStreamer.EmitSymbolValue(I, Size);
> -    else
> +
> +  // Grab the specific ranges for the compile units in the module.
> +  for (DenseMap<const MDNode *, CompileUnit *>::iterator I = CUMap.begin(),
> +                                                         E = CUMap.end();
> +       I != E; ++I) {
> +    CompileUnit *TheCU = I->second;
> +    unsigned ID = TheCU->getUniqueID();
> +
> +    // Emit a symbol so we can find the beginning of our ranges.
> +    Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("gnu_ranges", ID));
> +
> +    // Iterate over the misc ranges for the compile units in the module.
> +    const SmallVectorImpl<RangeSpanList *> &RangeLists = TheCU->getRangeLists();
> +    for (SmallVectorImpl<RangeSpanList *>::const_iterator
> +             I = RangeLists.begin(),
> +             E = RangeLists.end();
> +         I != E; ++I) {
> +      RangeSpanList *List = *I;
> +
> +      // Emit a symbol so we can find the beginning of the range.
> +      Asm->OutStreamer.EmitLabel(
> +          Asm->GetTempSymbol("debug_ranges", List->getIndex()));
> +
> +      for (SmallVectorImpl<RangeSpan>::const_iterator
> +               I = List->getRanges().begin(),
> +               E = List->getRanges().end();
> +           I != E; ++I) {
> +        RangeSpan Range = *I;
> +        // We occasionally have ranges without begin/end labels.
> +        // FIXME: Verify and fix.

Any test cases for that? Would be nice to have them committed and
XFAILed, ideally. (yeah, I don't often do this either, so hardly a
requirement)

> +        const MCSymbol *Begin = Range.getStart();
> +        const MCSymbol *End = Range.getEnd();
> +        Begin ? Asm->OutStreamer.EmitSymbolValue(Begin, Size)
> +              : Asm->OutStreamer.EmitIntValue(0, Size);
> +        End ? Asm->OutStreamer.EmitSymbolValue(End, Size)
> +            : Asm->OutStreamer.EmitIntValue(0, Size);
> +      }
> +
> +      // And terminate the list with two 0 values.
> +      Asm->OutStreamer.EmitIntValue(0, Size);
>        Asm->OutStreamer.EmitIntValue(0, Size);
> +    }
>    }
>  }
>
> @@ -3007,14 +3044,17 @@ CompileUnit *DwarfDebug::constructSkelet
>            DwarfGnuPubTypesSectionSym);
>    }
>
> -  // Flag if we've emitted any ranges and their location for the compile unit.
> -  if (DebugRangeSymbols.size()) {
> +  // Attribute if we've emitted any ranges and their location for the compile unit.
> +  if (CU->getRangeLists().size()) {

This looks like a !empty() test.

>      if (Asm->MAI->doesDwarfUseRelocationsAcrossSections())
> -      NewCU->addSectionLabel(Die, dwarf::DW_AT_GNU_ranges_base,
> -                             DwarfDebugRangeSectionSym);
> +      NewCU->addSectionLabel(
> +          Die, dwarf::DW_AT_GNU_ranges_base,
> +          Asm->GetTempSymbol("gnu_ranges", NewCU->getUniqueID()));
>      else
> -      NewCU->addUInt(Die, dwarf::DW_AT_GNU_ranges_base, dwarf::DW_FORM_data4,
> -                     0);
> +      NewCU->addSectionDelta(
> +          Die, dwarf::DW_AT_GNU_ranges_base,
> +          Asm->GetTempSymbol("gnu_ranges", NewCU->getUniqueID()),
> +          DwarfDebugRangeSectionSym);
>    }
>
>    SkeletonHolder.addUnit(NewCU);
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=196178&r1=196177&r2=196178&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Dec  2 18:45:45 2013
> @@ -409,8 +409,6 @@ class DwarfDebug {
>      DbgValueHistoryMap;
>    DbgValueHistoryMap DbgValues;
>
> -  SmallVector<const MCSymbol *, 8> DebugRangeSymbols;
> -
>    // Previous instruction's location information. This is used to determine
>    // label location to indicate scope boundries in dwarf debug info.
>    DebugLoc PrevInstLoc;
> @@ -437,6 +435,9 @@ class DwarfDebug {
>    // Counter for assigning globally unique IDs for CUs.
>    unsigned GlobalCUIndexCount;
>
> +  // Counter for assigning globally unique IDs for ranges.
> +  unsigned GlobalRangeCount;
> +
>    // Holder for the file specific debug information.
>    DwarfUnits InfoHolder;
>
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=196178&r1=196177&r2=196178&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Mon Dec  2 18:45:45 2013
> @@ -59,6 +59,10 @@ TypeUnit::TypeUnit(unsigned UID, DIE *D,
>  Unit::~Unit() {
>    for (unsigned j = 0, M = DIEBlocks.size(); j < M; ++j)
>      DIEBlocks[j]->~DIEBlock();
> +  for (SmallVectorImpl<RangeSpanList *>::iterator RI = getRangeLists().begin(),
> +                                                  RE = getRangeLists().end();
> +       RI != RE; ++RI)
> +    delete *RI;
>  }
>
>  /// createDIEEntry - Creates a new DIEEntry to be a proxy for a debug
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h?rev=196178&r1=196177&r2=196178&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.h Mon Dec  2 18:45:45 2013
> @@ -31,6 +31,31 @@ class ConstantInt;
>  class ConstantFP;
>  class DbgVariable;
>
> +// Data structure to hold a range for range lists.
> +class RangeSpan {

"RangeSpan" seems a bit redundant/confusing, unless I'm missing
something. "AddressRange", perhaps?

> +public:
> +  RangeSpan(MCSymbol *S, MCSymbol *E) : Start(S), End(E) {}
> +  const MCSymbol *getStart() { return Start; }
> +  const MCSymbol *getEnd() { return End; }
> +
> +private:
> +  const MCSymbol *Start, *End;
> +};
> +
> +class RangeSpanList {

I think the DWARF spec refers to each top level entry in the ranges
section as a "range set", possibly? Maybe reusing that terminology
would be helpful?

> +private:
> +  // Index for locating within the debug_range section this particular span.
> +  unsigned Index;
> +  // List of ranges.
> +  SmallVector<RangeSpan, 2> Ranges;
> +
> +public:
> +  RangeSpanList(unsigned Idx) : Index(Idx) {}
> +  unsigned getIndex() const { return Index; }
> +  const SmallVectorImpl<RangeSpan> &getRanges() const { return Ranges; }
> +  void addRange(RangeSpan Range) { Ranges.push_back(Range); }
> +};
> +
>  //===----------------------------------------------------------------------===//
>  /// Unit - This dwarf writer support class manages information associated
>  /// with a source file.
> @@ -92,6 +117,10 @@ protected:
>    /// corresponds to the MDNode mapped with the subprogram DIE.
>    DenseMap<DIE *, const MDNode *> ContainingTypeMap;
>
> +  // List of range lists for a given compile unit, separate from the ranges for
> +  // the CU itself.
> +  SmallVector<RangeSpanList *, 1> CURangeLists;
> +
>    // DIEValueAllocator - All DIEValues are allocated through this allocator.
>    BumpPtrAllocator DIEValueAllocator;
>
> @@ -132,6 +161,15 @@ public:
>    /// hasContent - Return true if this compile unit has something to write out.
>    bool hasContent() const { return !UnitDie->getChildren().empty(); }
>
> +  /// addRangeList - Add an address range list to the list of range lists.
> +  void addRangeList(RangeSpanList *Ranges) { CURangeLists.push_back(Ranges); }
> +
> +  /// getRangeLists - Get the vector of range lists.
> +  const SmallVectorImpl<RangeSpanList *> &getRangeLists() const {
> +    return CURangeLists;
> +  }
> +  SmallVectorImpl<RangeSpanList *> &getRangeLists() { return CURangeLists; }
> +
>    /// getParentContextString - Get a string containing the language specific
>    /// context for a global name.
>    std::string getParentContextString(DIScope Context) const;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits



More information about the llvm-commits mailing list