<div dir="ltr"><br><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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);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></blockquote><div><br></div><div>I had to change the above line slightly, to:</div><div>   return getOrCreateLexicalScope(IA);</div><div><br></div><div>(which essentially ends up mapping to: "return getOrCreateLexicalScope(IA->getScope(), IA->getInlinedAt());")</div><div><br></div><div>Otherwise, with the source file I tried, I the new recursive call to getOrCreateLexicalScope (where the IA->getScope() passed it was also in the NoDebug CU) ended up calling getOrCreateRegularScope, which asserted because Parent==nullptr and CurrentFnLexicalScope != nullptr.</div><div><br></div><div>Does my tweak make sense?</div><div><br></div><div>Teresa</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><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="gmail-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:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr" class="gmail-m_8251086938961588333gmail_msg"><div class="gmail_quote gmail-m_8251086938961588333gmail_msg"><div dir="ltr" class="gmail-m_8251086938961588333gmail_msg">On Tue, Feb 14, 2017 at 9:41 AM Teresa Johnson via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail-m_8251086938961588333gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="gmail-m_8251086938961588333gmail_msg"></div><blockquote class="gmail_quote gmail-m_8251086938961588333gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">tejohnson added inline comments.<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
================<br class="gmail-m_8251086938961588333gmail_msg">
Comment at: lib/CodeGen/LexicalScopes.cpp:<wbr>82<br class="gmail-m_8251086938961588333gmail_msg">
+      // Ignore locations linked from a NoDebug compile unit.<br class="gmail-m_8251086938961588333gmail_msg">
+      auto *SP = dyn_cast<DISubprogram>(MIDL-><wbr>getScope());<br class="gmail-m_8251086938961588333gmail_msg">
+      if (SP && SP->getUnit()-><wbr>getEmissionKind() == DICompileUnit::NoDebug) {<br class="gmail-m_8251086938961588333gmail_msg">
----------------<br class="gmail-m_8251086938961588333gmail_msg">
dblaikie wrote:<br class="gmail-m_8251086938961588333gmail_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="gmail-m_8251086938961588333gmail_msg">
><br class="gmail-m_8251086938961588333gmail_msg">
> It should be an invariant/guaranteed that a DISubprogram is reached (& is the end of the scope chain), so something like:<br class="gmail-m_8251086938961588333gmail_msg">
><br class="gmail-m_8251086938961588333gmail_msg">
> oh, wait, no, here it is: <a href="http://llvm.org/docs/doxygen/html/classllvm_1_1DILocalScope.html#a747dd1690fc477a8d9638fa37a30ced8" rel="noreferrer" class="gmail-m_8251086938961588333gmail_msg" target="_blank">http://llvm.org/docs/doxygen/<wbr>html/classllvm_1_<wbr>1DILocalScope.html#<wbr>a747dd1690fc477a8d9638fa37a30c<wbr>ed8</a><br class="gmail-m_8251086938961588333gmail_msg">
><br class="gmail-m_8251086938961588333gmail_msg">
> so, I think, maybe:<br class="gmail-m_8251086938961588333gmail_msg">
><br class="gmail-m_8251086938961588333gmail_msg">
>   if (cast<DILocalScope>(MIDL-><wbr>getScope())->getSubprogram()-><wbr>getUnit()->getEmissionKind() == DICompileUnit::NoDebug) {<br class="gmail-m_8251086938961588333gmail_msg">
>     ...<br class="gmail-m_8251086938961588333gmail_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="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
      // Ignore locations linked from a NoDebug compile unit.<br class="gmail-m_8251086938961588333gmail_msg">
      auto *SP = MIDL->getScope()-><wbr>getSubprogram();<br class="gmail-m_8251086938961588333gmail_msg">
      if (SP->getUnit()-><wbr>getEmissionKind() == DICompileUnit::NoDebug) {<br class="gmail-m_8251086938961588333gmail_msg">
        PrevMI = &MInsn;<br class="gmail-m_8251086938961588333gmail_msg">
        continue;<br class="gmail-m_8251086938961588333gmail_msg">
      }<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
(Don't need to cast MIDL->getScope() since it returns a DILocalScope).</blockquote><div class="gmail-m_8251086938961588333gmail_msg"><br class="gmail-m_8251086938961588333gmail_msg"></div></div></div><div dir="ltr" class="gmail-m_8251086938961588333gmail_msg"><div class="gmail_quote gmail-m_8251086938961588333gmail_msg"><div class="gmail-m_8251086938961588333gmail_msg">Ah, handy!</div></div></div><div dir="ltr" class="gmail-m_8251086938961588333gmail_msg"><div class="gmail_quote gmail-m_8251086938961588333gmail_msg"><div class="gmail-m_8251086938961588333gmail_msg"> </div><blockquote class="gmail_quote gmail-m_8251086938961588333gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"> But I am *still* getting the original seg fault. Any idea what else I could be missing?<br class="gmail-m_8251086938961588333gmail_msg"></blockquote><div class="gmail-m_8251086938961588333gmail_msg"><br class="gmail-m_8251086938961588333gmail_msg"></div></div></div><div dir="ltr" class="gmail-m_8251086938961588333gmail_msg"><div class="gmail_quote gmail-m_8251086938961588333gmail_msg"><div class="gmail-m_8251086938961588333gmail_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="gmail-m_8251086938961588333gmail_msg"><div class="gmail_quote gmail-m_8251086938961588333gmail_msg"><div class="gmail-m_8251086938961588333gmail_msg"> </div><blockquote class="gmail_quote gmail-m_8251086938961588333gmail_msg" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
================<br class="gmail-m_8251086938961588333gmail_msg">
Comment at: test/DebugInfo/Generic/debug_<wbr>and_nodebug_CUs.ll:4<br class="gmail-m_8251086938961588333gmail_msg">
+<br class="gmail-m_8251086938961588333gmail_msg">
+; RUN: llc %s -o - | FileCheck %s<br class="gmail-m_8251086938961588333gmail_msg">
+; CHECK-DAG: # debug.c:<br class="gmail-m_8251086938961588333gmail_msg">
----------------<br class="gmail-m_8251086938961588333gmail_msg">
dblaikie wrote:<br class="gmail-m_8251086938961588333gmail_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="gmail-m_8251086938961588333gmail_msg">
Will do<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
<a href="https://reviews.llvm.org/D29765" rel="noreferrer" class="gmail-m_8251086938961588333gmail_msg" target="_blank">https://reviews.llvm.org/<wbr>D29765</a><br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
<br class="gmail-m_8251086938961588333gmail_msg">
</blockquote></div></div></blockquote></div></div></div></div>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="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:2px solid rgb(213,15,37)">Teresa Johnson |</td><td nowrap style="border-top:2px solid rgb(51,105,232)"> Software Engineer |</td><td nowrap style="border-top:2px solid rgb(0,153,57)"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top:2px solid rgb(238,178,17)"> 408-460-2413</td></tr></tbody></table></span></div>
</div></div>