<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Sep 10, 2013 at 11:47 AM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Testcase? How did you see this?<br></blockquote><div>This should not affect functionality :)</div>
<div>It basically swaps the order of creation of Scope DIE vs. creation of children.</div><div><br></div><div>In the process, we removed an early exit which only captures some cases of creating dangling children.</div><div>
<div>- // Early return to avoid creating dangling variable|scope DIEs.</div><div>- if (!Scope->getInlinedAt() && DS.isSubprogram() && Scope->isAbstractScope() &&</div><div>- !TheCU->getDIE(DS))</div>
<div>- return NULL;</div></div><div><br></div><div>Creation of unused children then removing them are not efficient and it may cause problems if a member</div><div>of DwarfDebug or CompileUnit holds a reference to the deleted DIE.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
For some of the patch:<br>
<div class="im"><br>
+bool DwarfDebug::isLexicalScopeDIENull(LexicalScope *Scope) {<br>
+ if (Scope->isAbstractScope())<br>
<br>
</div>Needs a block comment.<br></blockquote><div>I have comments at the header file, I will copy it over to the source file. </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class="im"><br>
+ SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();<br>
+ MCSymbol *End = getLabelAfterInsn(RI->second);<br>
+ return !End;<br>
<br>
</div>Where does this show up?<br></blockquote><div>I write up isLexicalScopeDIENull from constructLexicalScopeDIE. constructLexicalScopeDIE returns null</div><div>when End is null. Is that what you are asking?</div><div>
<br></div><div>Thanks,</div><div>Manman</div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span class=""><font color="#888888"><br>
-eric<br>
</font></span><div class=""><div class="h5"><br>
<br>
On Tue, Sep 10, 2013 at 11:40 AM, Manman Ren <<a href="mailto:manman.ren@gmail.com">manman.ren@gmail.com</a>> wrote:<br>
> Author: mren<br>
> Date: Tue Sep 10 13:40:41 2013<br>
> New Revision: 190421<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=190421&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=190421&view=rev</a><br>
> Log:<br>
> Debug Info: create scope children DIEs when the scope DIE is not null.<br>
><br>
> We try to create the scope children DIEs after we create the scope DIE. But<br>
> to avoid emitting empty lexical block DIE, we first check whether a scope<br>
> DIE is going to be null, then create the scope children if it is not null.<br>
> From the number of children, we decide whether to actually create the scope DIE.<br>
><br>
> This patch also removes an early exit which checks for a special condition.<br>
> It also removes deletion of un-used children DIEs that are generated<br>
> because we used to generate children DIEs before the scope DIE.<br>
><br>
> Deletion of un-used children DIEs may cause problem because we sometimes keep<br>
> created DIEs in a member variable of a CU.<br>
><br>
> Modified:<br>
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=190421&r1=190420&r2=190421&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=190421&r1=190420&r2=190421&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)<br>
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Sep 10 13:40:41 2013<br>
> @@ -424,18 +424,34 @@ DIE *DwarfDebug::updateSubprogramScopeDI<br>
> return SPDie;<br>
> }<br>
><br>
> +bool DwarfDebug::isLexicalScopeDIENull(LexicalScope *Scope) {<br>
> + if (Scope->isAbstractScope())<br>
> + return false;<br>
> +<br>
> + const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();<br>
> + if (Ranges.empty())<br>
> + return true;<br>
> +<br>
> + if (Ranges.size() > 1)<br>
> + return false;<br>
> +<br>
> + SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();<br>
> + MCSymbol *End = getLabelAfterInsn(RI->second);<br>
> + return !End;<br>
> +}<br>
> +<br>
> // Construct new DW_TAG_lexical_block for this scope and attach<br>
> // DW_AT_low_pc/DW_AT_high_pc labels.<br>
> DIE *DwarfDebug::constructLexicalScopeDIE(CompileUnit *TheCU,<br>
> LexicalScope *Scope) {<br>
> + if (isLexicalScopeDIENull(Scope))<br>
> + return 0;<br>
> +<br>
> DIE *ScopeDIE = new DIE(dwarf::DW_TAG_lexical_block);<br>
> if (Scope->isAbstractScope())<br>
> return ScopeDIE;<br>
><br>
> const SmallVectorImpl<InsnRange> &Ranges = Scope->getRanges();<br>
> - if (Ranges.empty())<br>
> - return 0;<br>
> -<br>
> // If we have multiple ranges, emit them into the range section.<br>
> if (Ranges.size() > 1) {<br>
> // .debug_range section has not been laid out yet. Emit offset in<br>
> @@ -460,8 +476,7 @@ DIE *DwarfDebug::constructLexicalScopeDI<br>
> SmallVectorImpl<InsnRange>::const_iterator RI = Ranges.begin();<br>
> MCSymbol *Start = getLabelBeforeInsn(RI->first);<br>
> MCSymbol *End = getLabelAfterInsn(RI->second);<br>
> -<br>
> - if (End == 0) return 0;<br>
> + assert(End && "End label should not be null!");<br>
><br>
> assert(Start->isDefined() && "Invalid starting label for an inlined scope!");<br>
> assert(End->isDefined() && "Invalid end label for an inlined scope!");<br>
> @@ -540,19 +555,9 @@ DIE *DwarfDebug::constructInlinedScopeDI<br>
> return ScopeDIE;<br>
> }<br>
><br>
> -// Construct a DIE for this scope.<br>
> -DIE *DwarfDebug::constructScopeDIE(CompileUnit *TheCU, LexicalScope *Scope) {<br>
> - if (!Scope || !Scope->getScopeNode())<br>
> - return NULL;<br>
> -<br>
> - DIScope DS(Scope->getScopeNode());<br>
> - // Early return to avoid creating dangling variable|scope DIEs.<br>
> - if (!Scope->getInlinedAt() && DS.isSubprogram() && Scope->isAbstractScope() &&<br>
> - !TheCU->getDIE(DS))<br>
> - return NULL;<br>
> -<br>
> - SmallVector<DIE *, 8> Children;<br>
> - DIE *ObjectPointer = NULL;<br>
> +DIE *DwarfDebug::createScopeChildrenDIE(CompileUnit *TheCU, LexicalScope *Scope,<br>
> + SmallVectorImpl<DIE*> &Children) {<br>
> + DIE *ObjectPointer = NULL;<br>
><br>
> // Collect arguments for current function.<br>
> if (LScopes.isCurrentFunctionScope(Scope))<br>
> @@ -576,6 +581,20 @@ DIE *DwarfDebug::constructScopeDIE(Compi<br>
> for (unsigned j = 0, M = Scopes.size(); j < M; ++j)<br>
> if (DIE *Nested = constructScopeDIE(TheCU, Scopes[j]))<br>
> Children.push_back(Nested);<br>
> + return ObjectPointer;<br>
> +}<br>
> +<br>
> +// Construct a DIE for this scope.<br>
> +DIE *DwarfDebug::constructScopeDIE(CompileUnit *TheCU, LexicalScope *Scope) {<br>
> + if (!Scope || !Scope->getScopeNode())<br>
> + return NULL;<br>
> +<br>
> + DIScope DS(Scope->getScopeNode());<br>
> +<br>
> + SmallVector<DIE *, 8> Children;<br>
> + DIE *ObjectPointer = NULL;<br>
> + bool ChildrenCreated = false;<br>
> +<br>
> DIE *ScopeDIE = NULL;<br>
> if (Scope->getInlinedAt())<br>
> ScopeDIE = constructInlinedScopeDIE(TheCU, Scope);<br>
> @@ -591,6 +610,12 @@ DIE *DwarfDebug::constructScopeDIE(Compi<br>
> ScopeDIE = updateSubprogramScopeDIE(TheCU, DS);<br>
> }<br>
> else {<br>
> + if (isLexicalScopeDIENull(Scope))<br>
> + return NULL;<br>
> + // We create children only when we know the scope DIE is not going to be<br>
> + // null.<br>
> + ObjectPointer = createScopeChildrenDIE(TheCU, Scope, Children);<br>
> + ChildrenCreated = true;<br>
> // There is no need to emit empty lexical block DIE.<br>
> std::pair<ImportedEntityMap::const_iterator,<br>
> ImportedEntityMap::const_iterator> Range = std::equal_range(<br>
> @@ -600,15 +625,19 @@ DIE *DwarfDebug::constructScopeDIE(Compi<br>
> if (Children.empty() && Range.first == Range.second)<br>
> return NULL;<br>
> ScopeDIE = constructLexicalScopeDIE(TheCU, Scope);<br>
> + assert(ScopeDIE && "Scope DIE should not be null.");<br>
> for (ImportedEntityMap::const_iterator i = Range.first; i != Range.second;<br>
> ++i)<br>
> constructImportedEntityDIE(TheCU, i->second, ScopeDIE);<br>
> }<br>
><br>
> if (!ScopeDIE) {<br>
> - std::for_each(Children.begin(), Children.end(), deleter<DIE>);<br>
> + assert(Children.empty() &&<br>
> + "We create children only when the scope DIE is not null.");<br>
> return NULL;<br>
> }<br>
> + if (!ChildrenCreated)<br>
> + ObjectPointer = createScopeChildrenDIE(TheCU, Scope, Children);<br>
><br>
> // Add children<br>
> for (SmallVectorImpl<DIE *>::iterator I = Children.begin(),<br>
><br>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=190421&r1=190420&r2=190421&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=190421&r1=190420&r2=190421&view=diff</a><br>
> ==============================================================================<br>
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)<br>
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Sep 10 13:40:41 2013<br>
> @@ -469,6 +469,9 @@ private:<br>
> /// \brief Construct new DW_TAG_lexical_block for this scope and<br>
> /// attach DW_AT_low_pc/DW_AT_high_pc labels.<br>
> DIE *constructLexicalScopeDIE(CompileUnit *TheCU, LexicalScope *Scope);<br>
> + /// A helper function to check whether the DIE for a given Scope is going<br>
> + /// to be null.<br>
> + bool isLexicalScopeDIENull(LexicalScope *Scope);<br>
><br>
> /// \brief This scope represents inlined body of a function. Construct<br>
> /// DIE to represent this concrete inlined copy of the function.<br>
> @@ -476,6 +479,9 @@ private:<br>
><br>
> /// \brief Construct a DIE for this scope.<br>
> DIE *constructScopeDIE(CompileUnit *TheCU, LexicalScope *Scope);<br>
> + /// A helper function to create children of a Scope DIE.<br>
> + DIE *createScopeChildrenDIE(CompileUnit *TheCU, LexicalScope *Scope,<br>
> + SmallVectorImpl<DIE*> &Children);<br>
><br>
> /// \brief Emit initial Dwarf sections with a label at the start of each one.<br>
> void emitSectionLabels();<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>