<div dir="ltr">Ah, sure - if it comes from two different calls (even to the same function) there's no inlined scope that would be correct, so it should be dropped.</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Feb 14, 2017 at 11:03 AM Taewook Oh <<a href="mailto:twoh@fb.com">twoh@fb.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="white" lang="EN-US" link="blue" vlink="purple" class="gmail_msg">
<div class="m_-4787197619545369882WordSection1 gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:Calibri" class="gmail_msg">I think I was unclear. What I meant was that it would be better if the merged and hoisted call instruction has a scope of “@test, line: 0, column: 0”, not “@bar, line: 0, column: 0”.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:Calibri" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:Calibri" class="gmail_msg">Thanks,<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:Calibri" class="gmail_msg">Taewook<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:Calibri" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
<div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in" class="gmail_msg">
<p class="MsoNormal gmail_msg"><b class="gmail_msg"><span style="font-family:Calibri;color:black" class="gmail_msg">From: </span>
</b><span style="font-family:Calibri;color:black" class="gmail_msg">David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>><br class="gmail_msg">
<b class="gmail_msg">Date: </b>Tuesday, February 14, 2017 at 10:49 AM<br class="gmail_msg">
<b class="gmail_msg">To: </b>"<a href="mailto:reviews%2BD29833%2Bpublic%2Bd0b46dd83e56e03f@reviews.llvm.org" class="gmail_msg" target="_blank">reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org</a>" <<a href="mailto:reviews%2BD29833%2Bpublic%2Bd0b46dd83e56e03f@reviews.llvm.org" class="gmail_msg" target="_blank">reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org</a>>, "<a href="mailto:aprantl@apple.com" class="gmail_msg" target="_blank">aprantl@apple.com</a>" <<a href="mailto:aprantl@apple.com" class="gmail_msg" target="_blank">aprantl@apple.com</a>>, "<a href="mailto:paul.robinson@sony.com" class="gmail_msg" target="_blank">paul.robinson@sony.com</a>" <<a href="mailto:paul.robinson@sony.com" class="gmail_msg" target="_blank">paul.robinson@sony.com</a>>, "<a href="mailto:danielcdh@gmail.com" class="gmail_msg" target="_blank">danielcdh@gmail.com</a>" <<a href="mailto:danielcdh@gmail.com" class="gmail_msg" target="_blank">danielcdh@gmail.com</a>>,
"<a href="mailto:echristo@gmail.com" class="gmail_msg" target="_blank">echristo@gmail.com</a>" <<a href="mailto:echristo@gmail.com" class="gmail_msg" target="_blank">echristo@gmail.com</a>>, "<a href="mailto:rnk@google.com" class="gmail_msg" target="_blank">rnk@google.com</a>" <<a href="mailto:rnk@google.com" class="gmail_msg" target="_blank">rnk@google.com</a>>, "<a href="mailto:rob.lougher@gmail.com" class="gmail_msg" target="_blank">rob.lougher@gmail.com</a>" <<a href="mailto:rob.lougher@gmail.com" class="gmail_msg" target="_blank">rob.lougher@gmail.com</a>><br class="gmail_msg">
<b class="gmail_msg">Cc: </b>Taewook Oh <<a href="mailto:twoh@fb.com" class="gmail_msg" target="_blank">twoh@fb.com</a>>, "<a href="mailto:andrea.dibiagio@gmail.com" class="gmail_msg" target="_blank">andrea.dibiagio@gmail.com</a>" <<a href="mailto:andrea.dibiagio@gmail.com" class="gmail_msg" target="_blank">andrea.dibiagio@gmail.com</a>>, "<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>" <<a href="mailto:llvm-commits@lists.llvm.org" class="gmail_msg" target="_blank">llvm-commits@lists.llvm.org</a>>, Mehdi AMINI <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>><br class="gmail_msg">
<b class="gmail_msg">Subject: </b>Re: [PATCH] D29833: Improve the API of DILocation::getMergedLocation()<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div></div></div><div bgcolor="white" lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_-4787197619545369882WordSection1 gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg">Ah, sure - we can't/shouldn't remove any inline scope information (if it's unambiguous - if we hoisted from two different function calls we should probably drop the inlining scope information in the same way that we drop the ambiguous locations,
unfortunately), but block scopes may or may not be helpful to preserve. I'm not sure what the right answer is there.<u class="gmail_msg"></u><u class="gmail_msg"></u></p>
</div>
<p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg">On Tue, Feb 14, 2017 at 10:44 AM Taewook Oh via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<u class="gmail_msg"></u><u class="gmail_msg"></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" class="gmail_msg">
<p class="MsoNormal gmail_msg" style="margin-bottom:12.0pt">twoh added inline comments.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
================<br class="gmail_msg">
Comment at: lib/IR/Instruction.cpp:664-668<br class="gmail_msg">
+ if (Loc && isa<CallInst>(this)) {<br class="gmail_msg">
+ DbgLoc = DILocation::get(Loc->getContext(), 0, 0, Loc->getScope(),<br class="gmail_msg">
+ Loc->getInlinedAt());<br class="gmail_msg">
+ return;<br class="gmail_msg">
+ }<br class="gmail_msg">
----------------<br class="gmail_msg">
dblaikie wrote:<br class="gmail_msg">
> I'm not sure we can get the scope right, though - since we're potentially moving this location across scopes. It's going to be jumpy/create a difficult range in the scope no matter what, really...<br class="gmail_msg">
><br class="gmail_msg">
> Any ideas? Take the scope from a nearby location at the destination, if possible? (doesn't really reflect reality, but nothing will - this would at least mean a chance of not punching holes in/making islands in scopes that would necessitate a DW_AT_ranges/more
verbose description/etc)<br class="gmail_msg">
It would be hard to get the precise scope anyway, but I think it might be worth to consider the inlining context. Please take a look at the following test case:<br class="gmail_msg">
<br class="gmail_msg">
```<br class="gmail_msg">
define i32 @bar() !dbg !6 {<br class="gmail_msg">
entry:<br class="gmail_msg">
%call = tail call i32 @foo(), !dbg !8<br class="gmail_msg">
ret i32 %call, !dbg !9<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
define i32 @test(i32 %a, i32 %b) !dbg !10 {<br class="gmail_msg">
entry:<br class="gmail_msg">
%tobool = icmp ne i32 %a, 0, !dbg !11<br class="gmail_msg">
br i1 %tobool, label %if.then, label %if.else, !dbg !11<br class="gmail_msg">
<br class="gmail_msg">
if.then: ; preds = %entry<br class="gmail_msg">
%call.i = call i32 @foo(), !dbg !12<br class="gmail_msg">
%sub = sub nsw i32 %b, %call.i, !dbg !14<br class="gmail_msg">
br label %if.end, !dbg !15<br class="gmail_msg">
<br class="gmail_msg">
if.else: ; preds = %entry<br class="gmail_msg">
%call1 = call i32 @foo(), !dbg !16<br class="gmail_msg">
%sub2 = sub nsw i32 %b, %call1, !dbg !17<br class="gmail_msg">
br label %if.end<br class="gmail_msg">
<br class="gmail_msg">
if.end: ; preds = %if.else, %if.then<br class="gmail_msg">
%b.addr.0 = phi i32 [ %sub, %if.then ], [ %sub2, %if.else ]<br class="gmail_msg">
ret i32 %b.addr.0, !dbg !18<br class="gmail_msg">
}<br class="gmail_msg">
```<br class="gmail_msg">
<br class="gmail_msg">
where<br class="gmail_msg">
<br class="gmail_msg">
```<br class="gmail_msg">
!6 = distinct !DISubprogram(name: "bar", ...)<br class="gmail_msg">
!10 = distinct !DISubprogram(name: "test", ...)<br class="gmail_msg">
!12 = !DILocation(line: 4, column: 10, scope: !6, inlinedAt: !13)<br class="gmail_msg">
!16 = !DILocation(line: 11, column: 10, scope: !10)<br class="gmail_msg">
```<br class="gmail_msg">
<br class="gmail_msg">
Here the call instruction in %if.then block is actually inlined from @bar, and if we pickup the scope from that instruction, the final code would be look something like below:<br class="gmail_msg">
<br class="gmail_msg">
```<br class="gmail_msg">
define i32 @test(i32 %a, i32 %b) !dbg !10 {<br class="gmail_msg">
entry:<br class="gmail_msg">
%call.i = call i32 @foo(), !dbg !11<br class="gmail_msg">
%sub = sub nsw i32 %b, %call.i, !dbg !13<br class="gmail_msg">
ret i32 %sub, !dbg !14<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
!6 = distinct !DISubprogram(name: "bar", ...)<br class="gmail_msg">
!11 = !DILocation(line: 0, column: 0, scope: !6, inlinedAt: !12)<br class="gmail_msg">
```<br class="gmail_msg">
<br class="gmail_msg">
As the call instructions are merged and hoisted inside test not bar, it would be awkward if the merged call instruction has a scope of bar, though the debug location is still just (line: 0, column: 0).<br class="gmail_msg">
<br class="gmail_msg">
I missed the discussion in llvm-dev and implemented a patch for the same issue considering inline scope (D29927). Maybe worth take a look.<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
Repository:<br class="gmail_msg">
rL LLVM<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D29833&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=9Leqrn73ABLnKnbnINVXAvQ8dotwlC-Yd0CdOm037Mo&s=_IcWlv6Hg_yo5fDdIiU0jfSUExVmbDNqlChJLK--7Y4&e=" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29833</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<u class="gmail_msg"></u><u class="gmail_msg"></u></p>
</blockquote>
</div>
</div></div></blockquote></div>