<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 25, 2017 at 5:58 PM Robinson, Paul <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-3106831915613383573WordSection1">
<p class="MsoNormal" style="margin-left:.5in">Beyond that we get to the zero location - but in what scope?<span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u><u></u></span></p>
<p class="MsoNormal"><a name="m_-3106831915613383573__MailEndCompose"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></a></p>
</div></div><div lang="EN-US" link="blue" vlink="purple"><div class="m_-3106831915613383573WordSection1"><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">The scope of where it's hoisted into; in this case, the scope of the branch instruction. Which could be a parent of the common parent of the hoisted instruction,
but so what. The scope of a line-0 location is not especially relevant, </span></p></div></div></blockquote><div><br>Seems pretty important to me in terms of whether or not it's part of an inlined function (that's modeled with the same scope infrastructure), for example. (& also in terms of splitting ranges - which is mostly just about the size of the debug info - range fragments add more overhead, etc)<br><br>The branch instruction could be from a caller into two inlined calls, etc.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div class="m_-3106831915613383573WordSection1"><p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">and using the scope of where the instruction lands seems least disruptive.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d">--paulr<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d"><u></u> <u></u></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> llvm-commits [mailto:<a href="mailto:llvm-commits-bounces@lists.llvm.org" target="_blank">llvm-commits-bounces@lists.llvm.org</a>]
<b>On Behalf Of </b>David Blaikie via llvm-commits<br>
<b>Sent:</b> Friday, August 25, 2017 5:10 PM<br>
<b>To:</b> Adrian Prantl<br>
<b>Cc:</b> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<b>Subject:</b> Re: [llvm] r310985 - Merge debug info when hoist then-else code to if.<u></u><u></u></span></p>
</div>
</div></div></div></div><div lang="EN-US" link="blue" vlink="purple"><div class="m_-3106831915613383573WordSection1"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Fri, Aug 25, 2017 at 4:44 PM Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Aug 25, 2017, at 3:15 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">Adrian - what were we doing about cases where merged locations were on call instructions? I think this case is one of those, where if the instruction is a call, this may drop the location from the call, then inline through the call creating
corrupted debug info.<u></u><u></u></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
<div>
<div>
<div>
<p class="MsoNormal">I guess that in this case the only sane thing we can do is create a new artificial line 0 location in one of the two calls' scope. It's not pretty, but that;s the best I can come up with.<u></u><u></u></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><br>
A few options:<br>
<br>
Further up in the same file, the code avoids merging if the instruction is a call (originally r380995) SimplifyCFG.cpp:1282.<br>
<br>
So that's one option - do nothing on calls. But it's probably not ideal/wrong (since it's not either location)<br>
<br>
Beyond that we get to the zero location - but in what scope?<br>
The root scope of the function? (what if these two calls are both within a single inlined call? That'll punch a hole in the range which isn't entirely accurate) The nearest common parent scope to the two instructions (assuming they both have debug locations?
If one doesn't have a location, could we reliably use the other location? Probably not - so if one doesn't have a location we should use the zero location at the root of the current function for sure)<br>
Worth finding the common parent scope?<br>
Should this functionality be built into getMergedLocation?<br>
<u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- adrian<u></u><u></u></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><br>
<br>
<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
Here's a test case that seems to exercise/assert on that (but it asserts relatively late - would be handy to have a way to run the verifier between each optimization - maybe there is such an option?):<u></u><u></u></p>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">struct string {</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> ~string();</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">};</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">void f2();</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">void f1(int) { f2(); }</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">void run(int c) {</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> string body;</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> while (true) {</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> if (c)</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> f1(c);</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> else</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> f1(c);</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New""> }</span><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="font-family:"Courier New"">}</span><u></u><u></u></p>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Aug 15, 2017 at 6:56 PM Dehao Chen via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">Author: dehao<br>
Date: Tue Aug 15 18:55:26 2017<br>
New Revision: 310985<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=310985&view=rev" target="_blank">
http://llvm.org/viewvc/llvm-project?rev=310985&view=rev</a><br>
Log:<br>
Merge debug info when hoist then-else code to if.<br>
<br>
Summary: When we move then-else code to if, we need to merge its debug info, otherwise the hoisted instruction may have inaccurate debug info attached.<br>
<br>
Reviewers: aprantl, probinson, dblaikie, echristo, loladiro<br>
<br>
Reviewed By: aprantl<br>
<br>
Subscribers: sanjoy, llvm-commits<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D36778" target="_blank">
https://reviews.llvm.org/D36778</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll<br>
Modified:<br>
llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
<br>
Modified: llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=310985&r1=310984&r2=310985&view=diff" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp?rev=310985&r1=310984&r2=310985&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp (original)<br>
+++ llvm/trunk/lib/Transforms/Utils/SimplifyCFG.cpp Tue Aug 15 18:55:26 2017<br>
@@ -1336,6 +1336,8 @@ HoistTerminator:<br>
I2->replaceAllUsesWith(NT);<br>
NT->takeName(I1);<br>
}<br>
+ NT->setDebugLoc(DILocation::getMergedLocation(<br>
+ I1->getDebugLoc(), I2->getDebugLoc()));<br>
<br>
IRBuilder<NoFolder> Builder(NT);<br>
// Hoisting one of the terminators from our successor is a great thing.<br>
<br>
Added: llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll?rev=310985&view=auto" target="_blank">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll?rev=310985&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll (added)<br>
+++ llvm/trunk/test/Transforms/SimplifyCFG/min_dbginfo.ll Tue Aug 15 18:55:26 2017<br>
@@ -0,0 +1,39 @@<br>
+; RUN: opt < %s -simplifycfg -S | FileCheck %s<br>
+<br>
+; Checks if the debug info is removed for the "select" instruction.<br>
+; CHECK: cmp {{.*}} !dbg<br>
+; CHECK-NOT: select {{.*}} !dbg<br>
+define i32 @min(i32 %a, i32 %b) {<br>
+entry:<br>
+ %cmp = icmp slt i32 %a, %b, !dbg !9<br>
+ br i1 %cmp, label %if.then, label %if.else, !dbg !10<br>
+<br>
+if.then:<br>
+ br label %return, !dbg !11<br>
+<br>
+if.else:<br>
+ br label %return, !dbg !12<br>
+<br>
+return:<br>
+ %retval.0 = phi i32 [ %a, %if.then ], [ %b, %if.else ]<br>
+ ret i32 %retval.0, !dbg !13<br>
+}<br>
+<br>
+!<a href="http://llvm.dbg.cu/" target="_blank">llvm.dbg.cu</a> = !{!0}<br>
+!llvm.module.flags = !{!3, !4, !5}<br>
+!llvm.ident = !{!6}<br>
+<br>
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 6.0.0 (trunk 310792)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)<br>
+!1 = !DIFile(filename: "<a href="http://min.cc" target="_blank">min.cc</a>", directory: "/")<br>
+!2 = !{}<br>
+!3 = !{i32 2, !"Dwarf Version", i32 4}<br>
+!4 = !{i32 2, !"Debug Info Version", i32 3}<br>
+!5 = !{i32 1, !"wchar_size", i32 4}<br>
+!6 = !{!"clang version 6.0.0 (trunk 310792)"}<br>
+!7 = distinct !DISubprogram(name: "min", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, flags: DIFlagPrototyped, isOptimized: true, unit: !0, variables: !2)<br>
+!8 = !DISubroutineType(types: !2)<br>
+!9 = !DILocation(line: 4, column: 8, scope: !7)<br>
+!10 = !DILocation(line: 4, column: 6, scope: !7)<br>
+!11 = !DILocation(line: 5, column: 3, scope: !7)<br>
+!12 = !DILocation(line: 7, column: 3, scope: !7)<br>
+!13 = !DILocation(line: 9, column: 1, scope: !7)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><u></u><u></u></p>
</blockquote>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div></div></div></blockquote></div></div>