<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Title" content="">
<meta name="Keywords" content="">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:"맑은 고딕";}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:Calibri;
color:windowtext;}
span.msoIns
{mso-style-type:export-only;
mso-style-name:"";
text-decoration:underline;
color:teal;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style>
</head>
<body bgcolor="white" lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:Calibri">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”.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:Calibri"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:Calibri">Thanks,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:Calibri">Taewook<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:Calibri"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-family:Calibri;color:black">From: </span>
</b><span style="font-family:Calibri;color:black">David Blaikie <dblaikie@gmail.com><br>
<b>Date: </b>Tuesday, February 14, 2017 at 10:49 AM<br>
<b>To: </b>"reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org" <reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org>, "aprantl@apple.com" <aprantl@apple.com>, "paul.robinson@sony.com" <paul.robinson@sony.com>, "danielcdh@gmail.com" <danielcdh@gmail.com>,
"echristo@gmail.com" <echristo@gmail.com>, "rnk@google.com" <rnk@google.com>, "rob.lougher@gmail.com" <rob.lougher@gmail.com><br>
<b>Cc: </b>Taewook Oh <twoh@fb.com>, "andrea.dibiagio@gmail.com" <andrea.dibiagio@gmail.com>, "llvm-commits@lists.llvm.org" <llvm-commits@lists.llvm.org>, Mehdi AMINI <mehdi.amini@apple.com><br>
<b>Subject: </b>Re: [PATCH] D29833: Improve the API of DILocation::getMergedLocation()<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">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.<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Tue, Feb 14, 2017 at 10:44 AM Taewook Oh via Phabricator <<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.org</a>> wrote:<o:p></o:p></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" style="margin-bottom:12.0pt">twoh added inline comments.<br>
<br>
<br>
================<br>
Comment at: lib/IR/Instruction.cpp:664-668<br>
+ if (Loc && isa<CallInst>(this)) {<br>
+ DbgLoc = DILocation::get(Loc->getContext(), 0, 0, Loc->getScope(),<br>
+ Loc->getInlinedAt());<br>
+ return;<br>
+ }<br>
----------------<br>
dblaikie wrote:<br>
> 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>
><br>
> 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>
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>
<br>
```<br>
define i32 @bar() !dbg !6 {<br>
entry:<br>
%call = tail call i32 @foo(), !dbg !8<br>
ret i32 %call, !dbg !9<br>
}<br>
<br>
define i32 @test(i32 %a, i32 %b) !dbg !10 {<br>
entry:<br>
%tobool = icmp ne i32 %a, 0, !dbg !11<br>
br i1 %tobool, label %if.then, label %if.else, !dbg !11<br>
<br>
if.then: ; preds = %entry<br>
%call.i = call i32 @foo(), !dbg !12<br>
%sub = sub nsw i32 %b, %call.i, !dbg !14<br>
br label %if.end, !dbg !15<br>
<br>
if.else: ; preds = %entry<br>
%call1 = call i32 @foo(), !dbg !16<br>
%sub2 = sub nsw i32 %b, %call1, !dbg !17<br>
br label %if.end<br>
<br>
if.end: ; preds = %if.else, %if.then<br>
%b.addr.0 = phi i32 [ %sub, %if.then ], [ %sub2, %if.else ]<br>
ret i32 %b.addr.0, !dbg !18<br>
}<br>
```<br>
<br>
where<br>
<br>
```<br>
!6 = distinct !DISubprogram(name: "bar", ...)<br>
!10 = distinct !DISubprogram(name: "test", ...)<br>
!12 = !DILocation(line: 4, column: 10, scope: !6, inlinedAt: !13)<br>
!16 = !DILocation(line: 11, column: 10, scope: !10)<br>
```<br>
<br>
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>
<br>
```<br>
define i32 @test(i32 %a, i32 %b) !dbg !10 {<br>
entry:<br>
%call.i = call i32 @foo(), !dbg !11<br>
%sub = sub nsw i32 %b, %call.i, !dbg !13<br>
ret i32 %sub, !dbg !14<br>
}<br>
<br>
!6 = distinct !DISubprogram(name: "bar", ...)<br>
!11 = !DILocation(line: 0, column: 0, scope: !6, inlinedAt: !12)<br>
```<br>
<br>
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>
<br>
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>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<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=" target="_blank">https://reviews.llvm.org/D29833</a><br>
<br>
<br>
<o:p></o:p></p>
</blockquote>
</div>
</div>
</body>
</html>