<div dir="ltr">Great, thanks! Will give that a shot with my original test case too. Looks like I still need the change in extractLexicalScopes, for the single level of inline (NoDebug into debug), right?<div><br></div><div>Teresa</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 14, 2017 at 2:36 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">I think this is probably the right thing (patch attached, and text here for readability):<br><br><div>diff --git lib/CodeGen/LexicalScopes.cpp lib/CodeGen/LexicalScopes.cpp</div><div>index fc84ddb1f6b..31203689b3a 100644</div><div>--- lib/CodeGen/LexicalScopes.cpp</div><div>+++ lib/CodeGen/LexicalScopes.cpp</div><div>@@ -127,6 +127,9 @@ LexicalScope *LexicalScopes::<wbr>findLexicalScope(const DILocation *DL) {</div><div> LexicalScope *LexicalScopes::<wbr>getOrCreateLexicalScope(const DILocalScope *Scope,</div><div>                                                      const DILocation *IA) {</div><div>   if (IA) {</div><div>+    if (Scope->getSubprogram()-><wbr>getUnit()->getEmissionKind() ==</div><div>+        DICompileUnit::NoDebug)</div><div>+      return getOrCreateLexicalScope(IA-><wbr>getScope());</div><div>     // Create an abstract scope for inlined function.</div><div>     getOrCreateAbstractScope(<wbr>Scope);</div><div>     // Create an inlined scope for inlined function.<br><br>And here's my test case:<br><br><div><font face="monospace">$ cat a.cpp</font></div><div><font face="monospace">void f1();</font></div><div><font face="monospace">__attribute__((always_inline)) void f2() {</font></div><div><font face="monospace">  f1();</font></div><div><font face="monospace">}</font></div><div><font face="monospace">void f3();</font></div><div><font face="monospace">void f4() {</font></div><div><font face="monospace">  f3();</font></div><div><font face="monospace">}</font></div><div><font face="monospace">$ cat b.cpp</font></div><div><font face="monospace">void f2();</font></div><div><font face="monospace">__attribute__((always_inline)) void f3() {</font></div><div><font face="monospace">  f2();</font></div><div><font face="monospace">}</font></div></div><div><font face="monospace">$ clang++ -flto a.cpp -g -c</font></div><div><font face="monospace">$ clang++ -flto b.cpp -Rpass=inline -c</font></div><div><font face="monospace">$ llvm-link {a,b}.o -o - | opt -O2 - -o ab.bc</font></div><div><font face="monospace">$ llc ab.bc</font></div><br><br>It might be worth testing with more than one level of nodebug function in the middle (f2_1, f2_2) - my first pass I called "getOrCreateRegularScope" which would've been incorrect in that case, I think. (you could change it back to that, demonstrate that it breaks, and that using getOrCreateLexicalScope fixes it - if you like) - I'm not sure I'd bother committing such test cases - this one seems to suffice, but just for some general confidence in the change.<br><br>But it's probably good as-is/without worrying too much, I think.<br><br>- Dave<div><div class="h5"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 14, 2017 at 9:48 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class="m_-281078660425995141gmail_msg"><div class="gmail_quote m_-281078660425995141gmail_msg"><div dir="ltr" class="m_-281078660425995141gmail_msg">On Tue, Feb 14, 2017 at 9:41 AM Teresa Johnson via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="m_-281078660425995141gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="m_-281078660425995141gmail_msg"></div><blockquote class="gmail_quote m_-281078660425995141gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">tejohnson added inline comments.<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
================<br class="m_-281078660425995141gmail_msg">
Comment at: lib/CodeGen/LexicalScopes.cpp:<wbr>82<br class="m_-281078660425995141gmail_msg">
+      // Ignore locations linked from a NoDebug compile unit.<br class="m_-281078660425995141gmail_msg">
+      auto *SP = dyn_cast<DISubprogram>(MIDL-><wbr>getScope());<br class="m_-281078660425995141gmail_msg">
+      if (SP && SP->getUnit()-><wbr>getEmissionKind() == DICompileUnit::NoDebug) {<br class="m_-281078660425995141gmail_msg">
----------------<br class="m_-281078660425995141gmail_msg">
dblaikie wrote:<br class="m_-281078660425995141gmail_msg">
> 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.<br class="m_-281078660425995141gmail_msg">
><br class="m_-281078660425995141gmail_msg">
> It should be an invariant/guaranteed that a DISubprogram is reached (& is the end of the scope chain), so something like:<br class="m_-281078660425995141gmail_msg">
><br class="m_-281078660425995141gmail_msg">
> oh, wait, no, here it is: <a href="http://llvm.org/docs/doxygen/html/classllvm_1_1DILocalScope.html#a747dd1690fc477a8d9638fa37a30ced8" rel="noreferrer" class="m_-281078660425995141gmail_msg" target="_blank">http://llvm.org/docs/doxygen/<wbr>html/classllvm_1_<wbr>1DILocalScope.html#<wbr>a747dd1690fc477a8d9638fa37a30c<wbr>ed8</a><br class="m_-281078660425995141gmail_msg">
><br class="m_-281078660425995141gmail_msg">
> so, I think, maybe:<br class="m_-281078660425995141gmail_msg">
><br class="m_-281078660425995141gmail_msg">
>   if (cast<DILocalScope>(MIDL-><wbr>getScope())->getSubprogram()-><wbr>getUnit()->getEmissionKind() == DICompileUnit::NoDebug) {<br class="m_-281078660425995141gmail_msg">
>     ...<br class="m_-281078660425995141gmail_msg">
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:<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
      // Ignore locations linked from a NoDebug compile unit.<br class="m_-281078660425995141gmail_msg">
      auto *SP = MIDL->getScope()-><wbr>getSubprogram();<br class="m_-281078660425995141gmail_msg">
      if (SP->getUnit()-><wbr>getEmissionKind() == DICompileUnit::NoDebug) {<br class="m_-281078660425995141gmail_msg">
        PrevMI = &MInsn;<br class="m_-281078660425995141gmail_msg">
        continue;<br class="m_-281078660425995141gmail_msg">
      }<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
(Don't need to cast MIDL->getScope() since it returns a DILocalScope).</blockquote><div class="m_-281078660425995141gmail_msg"><br class="m_-281078660425995141gmail_msg"></div></div></div><div dir="ltr" class="m_-281078660425995141gmail_msg"><div class="gmail_quote m_-281078660425995141gmail_msg"><div class="m_-281078660425995141gmail_msg">Ah, handy!</div></div></div><div dir="ltr" class="m_-281078660425995141gmail_msg"><div class="gmail_quote m_-281078660425995141gmail_msg"><div class="m_-281078660425995141gmail_msg"> </div><blockquote class="gmail_quote m_-281078660425995141gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> But I am *still* getting the original seg fault. Any idea what else I could be missing?<br class="m_-281078660425995141gmail_msg"></blockquote><div class="m_-281078660425995141gmail_msg"><br class="m_-281078660425995141gmail_msg"></div></div></div><div dir="ltr" class="m_-281078660425995141gmail_msg"><div class="gmail_quote m_-281078660425995141gmail_msg"><div class="m_-281078660425995141gmail_msg">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.</div></div></div><div dir="ltr" class="m_-281078660425995141gmail_msg"><div class="gmail_quote m_-281078660425995141gmail_msg"><div class="m_-281078660425995141gmail_msg"> </div><blockquote class="gmail_quote m_-281078660425995141gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
================<br class="m_-281078660425995141gmail_msg">
Comment at: test/DebugInfo/Generic/debug_<wbr>and_nodebug_CUs.ll:4<br class="m_-281078660425995141gmail_msg">
+<br class="m_-281078660425995141gmail_msg">
+; RUN: llc %s -o - | FileCheck %s<br class="m_-281078660425995141gmail_msg">
+; CHECK-DAG: # debug.c:<br class="m_-281078660425995141gmail_msg">
----------------<br class="m_-281078660425995141gmail_msg">
dblaikie wrote:<br class="m_-281078660425995141gmail_msg">
> 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.<br class="m_-281078660425995141gmail_msg">
Will do<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
<a href="https://reviews.llvm.org/D29765" rel="noreferrer" class="m_-281078660425995141gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D29765</a><br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
<br class="m_-281078660425995141gmail_msg">
</blockquote></div></div></blockquote></div></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> 408-460-2413</td></tr></tbody></table></span></div>
</div>