[llvm] r190421 - Debug Info: create scope children DIEs when the scope DIE is not null.

Eric Christopher echristo at gmail.com
Wed Sep 11 13:02:39 PDT 2013


On Tue, Sep 10, 2013 at 3:17 PM, Manman Ren <manman.ren at gmail.com> wrote:
> |Yep. I'm still wondering where this shows up. :\
>
> What does "this" mean here? :)
> Before the change, when we run llc on
> CodeGen/X86/stack-protector-dbginfo.ll, we constructed scope children and
> then later removed them.
> If you set a breakpoint at "std::for_each(Children.begin(), Children.end(),
> deleter<DIE>)"
>

Interesting, but no, I was wondering about:

>>> >> +  MCSymbol *End = getLabelAfterInsn(RI->second);
>>> >> +  return !End;

Where we've got an end that doesn't have a label after it.

-eric

> Manman
>
>
>
> On Tue, Sep 10, 2013 at 2:30 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>>
>> On Tue, Sep 10, 2013 at 2:17 PM, Eric Christopher <echristo at gmail.com>
>> wrote:
>>>
>>> >> Testcase? How did you see this?
>>> >
>>> > This should not affect functionality :)
>>>
>>> Oh good. I was hoping not. How did you come across this? I'm curious.
>>>
>>> > It basically swaps the order of creation of Scope DIE vs. creation of
>>> > children.
>>> >
>>>
>>> Yeah. It could use a lot more documentation.
>>>
>>> > In the process, we removed an early exit which only captures some cases
>>> > of
>>> > creating dangling children.
>>> > -  // Early return to avoid creating dangling variable|scope DIEs.
>>> > -  if (!Scope->getInlinedAt() && DS.isSubprogram() &&
>>> > Scope->isAbstractScope() &&
>>> > -      !TheCU->getDIE(DS))
>>> > -    return NULL;
>>> >
>>> > Creation of unused children then removing them are not efficient and it
>>> > may
>>> > cause problems if a member
>>> > of DwarfDebug or CompileUnit holds a reference to the deleted DIE.
>>> >
>>>
>>> Makes sense, but I think only if you actually create them :)
>>>
>>> >>
>>> >>
>>> >> For some of the patch:
>>> >>
>>> >> +bool DwarfDebug::isLexicalScopeDIENull(LexicalScope *Scope) {
>>> >> +  if (Scope->isAbstractScope())
>>> >>
>>> >> Needs a block comment.
>>> >
>>> > I have comments at the header file, I will copy it over to the source
>>> > file.
>>>
>>> Please be more detailed than in the header file.
>>>
>>> >>
>>> >>
>>> >> +  SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();
>>> >> +  MCSymbol *End = getLabelAfterInsn(RI->second);
>>> >> +  return !End;
>>> >>
>>> >> Where does this show up?
>>> >
>>> > I write up isLexicalScopeDIENull from constructLexicalScopeDIE.
>>> > constructLexicalScopeDIE returns null
>>> > when End is null. Is that what you are asking?
>>> >
>>>
>>> Yep. I'm still wondering where this shows up. :\
>>
>>
>> Yeah, I'm rather curious about this. Given that we don't have a list of
>> lexical scope children in their parent scope, the only reason a lexical
>> scope even stays alive is by virtue of its children (instructions,
>> variables, etc) staying alive within it - if all the children go away, the
>> lexical scope should be dropped or otherwise never be seen/reached.
>>
>>>
>>>
>>> -eric
>>>
>>> > Thanks,
>>> > Manman
>>> >>
>>> >>
>>> >> -eric
>>> >>
>>> >>
>>> >> On Tue, Sep 10, 2013 at 11:40 AM, Manman Ren <manman.ren at gmail.com>
>>> >> wrote:
>>> >> > Author: mren
>>> >> > Date: Tue Sep 10 13:40:41 2013
>>> >> > New Revision: 190421
>>> >> >
>>> >> > URL: http://llvm.org/viewvc/llvm-project?rev=190421&view=rev
>>> >> > Log:
>>> >> > Debug Info: create scope children DIEs when the scope DIE is not
>>> >> > null.
>>> >> >
>>> >> > We try to create the scope children DIEs after we create the scope
>>> >> > DIE.
>>> >> > But
>>> >> > to avoid emitting empty lexical block DIE, we first check whether a
>>> >> > scope
>>> >> > DIE is going to be null, then create the scope children if it is not
>>> >> > null.
>>> >> > From the number of children, we decide whether to actually create
>>> >> > the
>>> >> > scope DIE.
>>> >> >
>>> >> > This patch also removes an early exit which checks for a special
>>> >> > condition.
>>> >> > It also removes deletion of un-used children DIEs that are generated
>>> >> > because we used to generate children DIEs before the scope DIE.
>>> >> >
>>> >> > Deletion of un-used children DIEs may cause problem because we
>>> >> > sometimes
>>> >> > keep
>>> >> > created DIEs in a member variable of a CU.
>>> >> >
>>> >> > Modified:
>>> >> >     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>> >> >     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>> >> >
>>> >> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>> >> > URL:
>>> >> >
>>> >> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=190421&r1=190420&r2=190421&view=diff
>>> >> >
>>> >> >
>>> >> > ==============================================================================
>>> >> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>> >> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Sep 10
>>> >> > 13:40:41
>>> >> > 2013
>>> >> > @@ -424,18 +424,34 @@ DIE *DwarfDebug::updateSubprogramScopeDI
>>> >> >    return SPDie;
>>> >> >  }
>>> >> >
>>> >> > +bool DwarfDebug::isLexicalScopeDIENull(LexicalScope *Scope) {
>>> >> > +  if (Scope->isAbstractScope())
>>> >> > +    return false;
>>> >> > +
>>> >> > +  const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();
>>> >> > +  if (Ranges.empty())
>>> >> > +    return true;
>>> >> > +
>>> >> > +  if (Ranges.size() > 1)
>>> >> > +    return false;
>>> >> > +
>>> >> > +  SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();
>>> >> > +  MCSymbol *End = getLabelAfterInsn(RI->second);
>>> >> > +  return !End;
>>> >> > +}
>>> >> > +
>>> >> >  // Construct new DW_TAG_lexical_block for this scope and attach
>>> >> >  // DW_AT_low_pc/DW_AT_high_pc labels.
>>> >> >  DIE *DwarfDebug::constructLexicalScopeDIE(CompileUnit *TheCU,
>>> >> >                                            LexicalScope *Scope) {
>>> >> > +  if (isLexicalScopeDIENull(Scope))
>>> >> > +    return 0;
>>> >> > +
>>> >> >    DIE *ScopeDIE = new DIE(dwarf::DW_TAG_lexical_block);
>>> >> >    if (Scope->isAbstractScope())
>>> >> >      return ScopeDIE;
>>> >> >
>>> >> >    const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();
>>> >> > -  if (Ranges.empty())
>>> >> > -    return 0;
>>> >> > -
>>> >> >    // If we have multiple ranges, emit them into the range section.
>>> >> >    if (Ranges.size() > 1) {
>>> >> >      // .debug_range section has not been laid out yet. Emit offset
>>> >> > in
>>> >> > @@ -460,8 +476,7 @@ DIE *DwarfDebug::constructLexicalScopeDI
>>> >> >    SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();
>>> >> >    MCSymbol *Start = getLabelBeforeInsn(RI->first);
>>> >> >    MCSymbol *End = getLabelAfterInsn(RI->second);
>>> >> > -
>>> >> > -  if (End == 0) return 0;
>>> >> > +  assert(End && "End label should not be null!");
>>> >> >
>>> >> >    assert(Start->isDefined() && "Invalid starting label for an
>>> >> > inlined
>>> >> > scope!");
>>> >> >    assert(End->isDefined() && "Invalid end label for an inlined
>>> >> > scope!");
>>> >> > @@ -540,19 +555,9 @@ DIE *DwarfDebug::constructInlinedScopeDI
>>> >> >    return ScopeDIE;
>>> >> >  }
>>> >> >
>>> >> > -// Construct a DIE for this scope.
>>> >> > -DIE *DwarfDebug::constructScopeDIE(CompileUnit *TheCU, LexicalScope
>>> >> > *Scope) {
>>> >> > -  if (!Scope || !Scope->getScopeNode())
>>> >> > -    return NULL;
>>> >> > -
>>> >> > -  DIScope DS(Scope->getScopeNode());
>>> >> > -  // Early return to avoid creating dangling variable|scope DIEs.
>>> >> > -  if (!Scope->getInlinedAt() && DS.isSubprogram() &&
>>> >> > Scope->isAbstractScope() &&
>>> >> > -      !TheCU->getDIE(DS))
>>> >> > -    return NULL;
>>> >> > -
>>> >> > -  SmallVector<DIE *, 8> Children;
>>> >> > -  DIE *ObjectPointer = NULL;
>>> >> > +DIE *DwarfDebug::createScopeChildrenDIE(CompileUnit *TheCU,
>>> >> > LexicalScope *Scope,
>>> >> > +                                        SmallVectorImpl<DIE*>
>>> >> > &Children) {
>>> >> > +    DIE *ObjectPointer = NULL;
>>> >> >
>>> >> >    // Collect arguments for current function.
>>> >> >    if (LScopes.isCurrentFunctionScope(Scope))
>>> >> > @@ -576,6 +581,20 @@ DIE *DwarfDebug::constructScopeDIE(Compi
>>> >> >    for (unsigned j = 0, M = Scopes.size(); j < M; ++j)
>>> >> >      if (DIE *Nested = constructScopeDIE(TheCU, Scopes[j]))
>>> >> >        Children.push_back(Nested);
>>> >> > +  return ObjectPointer;
>>> >> > +}
>>> >> > +
>>> >> > +// Construct a DIE for this scope.
>>> >> > +DIE *DwarfDebug::constructScopeDIE(CompileUnit *TheCU, LexicalScope
>>> >> > *Scope) {
>>> >> > +  if (!Scope || !Scope->getScopeNode())
>>> >> > +    return NULL;
>>> >> > +
>>> >> > +  DIScope DS(Scope->getScopeNode());
>>> >> > +
>>> >> > +  SmallVector<DIE *, 8> Children;
>>> >> > +  DIE *ObjectPointer = NULL;
>>> >> > +  bool ChildrenCreated = false;
>>> >> > +
>>> >> >    DIE *ScopeDIE = NULL;
>>> >> >    if (Scope->getInlinedAt())
>>> >> >      ScopeDIE = constructInlinedScopeDIE(TheCU, Scope);
>>> >> > @@ -591,6 +610,12 @@ DIE *DwarfDebug::constructScopeDIE(Compi
>>> >> >        ScopeDIE = updateSubprogramScopeDIE(TheCU, DS);
>>> >> >    }
>>> >> >    else {
>>> >> > +    if (isLexicalScopeDIENull(Scope))
>>> >> > +      return NULL;
>>> >> > +    // We create children only when we know the scope DIE is not
>>> >> > going
>>> >> > to be
>>> >> > +    // null.
>>> >> > +    ObjectPointer = createScopeChildrenDIE(TheCU, Scope, Children);
>>> >> > +    ChildrenCreated = true;
>>> >> >      // There is no need to emit empty lexical block DIE.
>>> >> >      std::pair<ImportedEntityMap::const_iterator,
>>> >> >                ImportedEntityMap::const_iterator> Range =
>>> >> > std::equal_range(
>>> >> > @@ -600,15 +625,19 @@ DIE *DwarfDebug::constructScopeDIE(Compi
>>> >> >      if (Children.empty() && Range.first == Range.second)
>>> >> >        return NULL;
>>> >> >      ScopeDIE = constructLexicalScopeDIE(TheCU, Scope);
>>> >> > +    assert(ScopeDIE && "Scope DIE should not be null.");
>>> >> >      for (ImportedEntityMap::const_iterator i = Range.first; i !=
>>> >> > Range.second;
>>> >> >           ++i)
>>> >> >        constructImportedEntityDIE(TheCU, i->second, ScopeDIE);
>>> >> >    }
>>> >> >
>>> >> >    if (!ScopeDIE) {
>>> >> > -    std::for_each(Children.begin(), Children.end(), deleter<DIE>);
>>> >> > +    assert(Children.empty() &&
>>> >> > +           "We create children only when the scope DIE is not
>>> >> > null.");
>>> >> >      return NULL;
>>> >> >    }
>>> >> > +  if (!ChildrenCreated)
>>> >> > +    ObjectPointer = createScopeChildrenDIE(TheCU, Scope, Children);
>>> >> >
>>> >> >    // Add children
>>> >> >    for (SmallVectorImpl<DIE *>::iterator I = Children.begin(),
>>> >> >
>>> >> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>> >> > URL:
>>> >> >
>>> >> > http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=190421&r1=190420&r2=190421&view=diff
>>> >> >
>>> >> >
>>> >> > ==============================================================================
>>> >> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>> >> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Sep 10
>>> >> > 13:40:41
>>> >> > 2013
>>> >> > @@ -469,6 +469,9 @@ private:
>>> >> >    /// \brief Construct new DW_TAG_lexical_block for this scope and
>>> >> >    /// attach DW_AT_low_pc/DW_AT_high_pc labels.
>>> >> >    DIE *constructLexicalScopeDIE(CompileUnit *TheCU, LexicalScope
>>> >> > *Scope);
>>> >> > +  /// A helper function to check whether the DIE for a given Scope
>>> >> > is
>>> >> > going
>>> >> > +  /// to be null.
>>> >> > +  bool isLexicalScopeDIENull(LexicalScope *Scope);
>>> >> >
>>> >> >    /// \brief This scope represents inlined body of a function.
>>> >> > Construct
>>> >> >    /// DIE to represent this concrete inlined copy of the function.
>>> >> > @@ -476,6 +479,9 @@ private:
>>> >> >
>>> >> >    /// \brief Construct a DIE for this scope.
>>> >> >    DIE *constructScopeDIE(CompileUnit *TheCU, LexicalScope *Scope);
>>> >> > +  /// A helper function to create children of a Scope DIE.
>>> >> > +  DIE *createScopeChildrenDIE(CompileUnit *TheCU, LexicalScope
>>> >> > *Scope,
>>> >> > +                              SmallVectorImpl<DIE*> &Children);
>>> >> >
>>> >> >    /// \brief Emit initial Dwarf sections with a label at the start
>>> >> > of
>>> >> > each one.
>>> >> >    void emitSectionLabels();
>>> >> >
>>> >> >
>>> >> > _______________________________________________
>>> >> > 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
>>
>>
>



More information about the llvm-commits mailing list