On Wed, Apr 6, 2016 at 8:44 AM, Adrian Prantl <apra...@apple.com> wrote:
> > 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> 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. > Not sure whether it adds much more regularity or irregularity compared to having a null unit for the subprogram, really? Anyone looking at the subprogram for debug purposes is going to have to walk up and check that it's not a nodebug CU, the CU isn't really a CU (when looking at the code/IR/etc - it'll be a weird construct)... but I dunno. > > -- 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 >> >> >> >> _______________________________________________ >> llvm-commits mailing list >> llvm-comm...@lists.llvm.org >> 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