[PATCH] D29765: Handle link of NoDebug CU with a CU that has debug emission enabled

[David Blaikie via llvm-commits]

On Tue, Feb 14, 2017 at 9:41 AM Teresa Johnson via Phabricator <
reviews at reviews.llvm.org> wrote:

> tejohnson added inline comments.
>
>
> ================
> Comment at: lib/CodeGen/LexicalScopes.cpp:82
> +      // Ignore locations linked from a NoDebug compile unit.
> +      auto *SP = dyn_cast<DISubprogram>(MIDL->getScope());
> +      if (SP && SP->getUnit()->getEmissionKind() ==
> DICompileUnit::NoDebug) {
> ----------------
> dblaikie wrote:
> > This probably won't catch all cases - if the location was in a subscope
> within the function (eg: try adding a {} to the function which should add
> an extra scope). Walking the scope chain until a DISubprogram is probably
> needed here - I forget if we have a nice wrapped up utility for that. Don't
> see one at a glance.
> >
> > It should be an invariant/guaranteed that a DISubprogram is reached (&
> is the end of the scope chain), so something like:
> >
> > oh, wait, no, here it is:
> http://llvm.org/docs/doxygen/html/classllvm_1_1DILocalScope.html#a747dd1690fc477a8d9638fa37a30ced8
> >
> > so, I think, maybe:
> >
> >   if
> (cast<DILocalScope>(MIDL->getScope())->getSubprogram()->getUnit()->getEmissionKind()
> == DICompileUnit::NoDebug) {
> >     ...
> I had just found the DILocalScope::getSubprogram() helper, since I was
> still getting the original seg fault when I tried the original failing
> internal code with this change. So now I have:
>
>       // Ignore locations linked from a NoDebug compile unit.
>       auto *SP = MIDL->getScope()->getSubprogram();
>       if (SP->getUnit()->getEmissionKind() == DICompileUnit::NoDebug) {
>         PrevMI = &MInsn;
>         continue;
>       }
>
> (Don't need to cast MIDL->getScope() since it returns a DILocalScope).


Ah, handy!


> But I am *still* getting the original seg fault. Any idea what else I
> could be missing?
>

Not sure, no, sorry :/ I know reducing LTO test cases is difficult, but if
this works for the simple test case and doesn't in the original failing
situation it'll be good to understand why.


>
>
> ================
> Comment at: test/DebugInfo/Generic/debug_and_nodebug_CUs.ll:4
> +
> +; RUN: llc %s -o - | FileCheck %s
> +; CHECK-DAG: # debug.c:
> ----------------
> dblaikie wrote:
> > Include the steps for reproducing this test case (basically the RUN
> lines, and the original C source/steps for producing the IR, from the first
> version of the test) - helps to visualize/document/reproduce the situation.
> Will do
>
>
> https://reviews.llvm.org/D29765
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170214/05c3e216/attachment.html>