> On Apr 6, 2016, at 2:56 PM, David Blaikie <dblai...@gmail.com> wrote:
>
>
>
> On Wed, Apr 6, 2016 at 8:44 AM, Adrian Prantl <apra...@apple.com
> <mailto:apra...@apple.com>> wrote:
>
>> On Apr 6, 2016, at 8:16 AM, David Blaikie <dblai...@gmail.com
>> <mailto: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.
>
> Not sure whether it adds much more regularity or irregularity compared to
> having a null unit for the subprogram, really?
My thinking was that we could then have to easily enforceable rules:
1. The DISubprograms that are part of the type hierarchy (uniqued, and
isDefinition: false) *never* have a unit field.
2. The DISubprogram definitions (distinct) *must* have a unit field.
> 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.
After the pointer reversal we can add a DISubprogram::isNoDebug() convenience
accessor. For the runtime performance the extra indirection doesn’t matter
because in each of these places we already look up the parent CU.
From a practical point of view, implementing the null unit option is actually
quite a bit more work, given how much CGDebugInfo and DIBuilder currently
depend on having a CU. With enough effort this can be cleaned up as well and
this patch doesn’t prevent us from doing this in the future.
-- adrian
>
>
> -- 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
