> On Apr 6, 2016, at 8:16 AM, David Blaikie <dblai...@gmail.com> wrote:
> 
> 
> 
> On Tue, Apr 5, 2016 at 10:17 PM, Eric Christopher via llvm-commits 
> <llvm-comm...@lists.llvm.org <mailto:llvm-comm...@lists.llvm.org>> wrote:
> echristo added inline comments.
> 
> ================
> Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:477-479
> @@ -476,2 +476,5 @@
> +  unsigned DebugCUs = 0;
>    for (MDNode *N : CU_Nodes->operands()) {
>      auto *CUNode = cast<DICompileUnit>(N);
> +    if (CUNode->getEmissionKind() == DICompileUnit::NoDebug)
> +      continue;
> ----------------
> Instead of this pattern would it make more sense to have an iterator over the 
> nodes that checks for !NoDebug?
> 
> Yeah, that sounds like something that might be nice.
> 
> Adrian - I recall one of your initial ideas was to just have a different 
> named metadata node for these subprograms.

Huh. I think I had this idea in the context of collectDeadVariables (to collect 
dead subprograms), but I see how it would apply here, too.

> What was the motivation for not going with that plan? (which would've meant 
> these sort of loops would've remained OK, and only the loops in the verifier 
> and such would need to examine both nodes?) Perhaps this relates to your 
> comment about LTOing debug+nodebug CUs? I didn't quite follow what was wrong 
> with that in the first place & how this addresses it, could you explain it?

Most of the patches I’ve been committing recently are in preparation reversing 
the ownership between DICompileUnit and DISubprogram (which will help ThinLTO 
to lazy-load debug info).
DICompileUnit will no longer have a list of “subprograms:" and instead each 
DISubprogram will have a “unit:" field that points to the owning CU.

While it would be possible to leave the unit field empty and have a NamedMDNode 
hold on to these subprograms, having a mandatory “unit:" point to a CU that is 
explicitly marked NoDebug is a more regular and readable representation. It’s 
straightforward to verify and easier to debug especially in the LTO case.

-- adrian

>  
> 
> ================
> Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.cpp:1127-1128
> @@ -1115,3 +1126,4 @@
>    if (!MMI->hasDebugInfo() || LScopes.empty() ||
> -      !MF->getFunction()->getSubprogram()) {
> +      !MF->getFunction()->getSubprogram() ||
> +      !SPMap.lookup(MF->getFunction()->getSubprogram())) {
>      // If we don't have a lexical scope for this function then there will
> ----------------
> Comment.
> 
> 
> Repository:
>   rL LLVM
> 
> http://reviews.llvm.org/D18808 <http://reviews.llvm.org/D18808>
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-comm...@lists.llvm.org <mailto:llvm-comm...@lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits 
> <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to