[llvm] r190421 - Debug Info: create scope children DIEs when the scope DIE is not null.
Manman Ren
manman.ren at gmail.com
Wed Sep 11 13:35:05 PDT 2013
On Wed, Sep 11, 2013 at 1:02 PM, Eric Christopher <echristo at gmail.com>wrote:
> 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.
>
I am not sure about that :)
I moved that part from constructLexicalScopeDIE, if you think it is not
going to happen, we can put an assert there.
Manman
>
> -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
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130911/495b7261/attachment.html>
More information about the llvm-commits
mailing list