[PATCH] D18808: Use the NoDebug emission kind to identify compile units that no debug info should be created from.

[David Blaikie via cfe-commits]

On Wed, Apr 6, 2016 at 8:44 AM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Apr 6, 2016, at 8:16 AM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Tue, Apr 5, 2016 at 10:17 PM, Eric Christopher via llvm-commits <
> llvm-commits at 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-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160406/3c028ebb/attachment-0001.html>