<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Feb 23, 2017 at 3:08 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 bgcolor="white" lang="EN-US" link="blue" vlink="purple" class="gmail_msg">
<div class="m_7649363730575125755WordSection1 gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d" class="gmail_msg">For DWARF, all "line 0" records are semantically equivalent. However there is a size cost in the encoded line table if an explicit "line 0" has a different
file (or column) than the previous or following line record.</span></p></div></div></blockquote><div><br></div><div>Yep, also a size cost if we encode it in a different scope from teh one nearby - could cause scopes to have holes and thus need DW_AT_ranges, etc... </div><div> </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_7649363730575125755WordSection1 gmail_msg"><p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d" 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","sans-serif";color:#1f497d" class="gmail_msg">--paulr<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
<p class="MsoNormal gmail_msg"><a name="m_7649363730575125755__MailEndCompose" class="gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1f497d" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></a></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt" class="gmail_msg">
<div class="gmail_msg">
<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-size:10.0pt;font-family:"Tahoma","sans-serif"" class="gmail_msg">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"" class="gmail_msg"> Taewook Oh [mailto:<a href="mailto:twoh@fb.com" class="gmail_msg" target="_blank">twoh@fb.com</a>]
<br class="gmail_msg">
<b class="gmail_msg">Sent:</b> Thursday, February 23, 2017 12:02 PM<br class="gmail_msg">
<b class="gmail_msg">To:</b> Adrian Prantl; David Blaikie<br class="gmail_msg">
<b class="gmail_msg">Cc:</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: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: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>; Robinson, Paul; <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:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a></span></p></div></div></div></div></div><div bgcolor="white" lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_7649363730575125755WordSection1 gmail_msg"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt" class="gmail_msg"><div class="gmail_msg"><div style="border:none;border-top:solid #b5c4df 1.0pt;padding:3.0pt 0in 0in 0in" class="gmail_msg"><p class="MsoNormal gmail_msg"><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"" class="gmail_msg"><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></div><div bgcolor="white" lang="EN-US" link="blue" vlink="purple" class="gmail_msg"><div class="m_7649363730575125755WordSection1 gmail_msg"><div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt" class="gmail_msg">
<p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
<p class="MsoNormal gmail_msg"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"" class="gmail_msg">I think the existing implementation of getMergedLocation implies that if two debug locations can be discriminated, we should return line:0 location than keep the original
location. The gist of this patch is returning line:0 locations instead of nullptr for call instructions, I believe.
<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","sans-serif"" 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","sans-serif"" class="gmail_msg">Regarding API, I think “I1->setMergedDebugLoc(I1->getDebugLoc(), I2->getDebugLoc()” is better than “I1->setDebugLoc(DILocation::getMergedLocation(I1->getDebugLoc(), I2->getDebugLoc()))”,
because it is more succinct, and we can handle CallInst separately. <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","sans-serif"" 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","sans-serif"" class="gmail_msg">Another issue we’ve been discussed was what should be the “scope” of returning line:0 location, but I actually don’t have strong opinion about it, because with line:0 it
is already clear that the DebugLoc is dummy (Maybe I missed some point here). <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","sans-serif"" 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","sans-serif"" 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","sans-serif"" 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","sans-serif"" 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","sans-serif";color:black" class="gmail_msg">From:
</span></b><span style="font-family:"Calibri","sans-serif";color:black" class="gmail_msg"><<a href="mailto:aprantl@apple.com" class="gmail_msg" target="_blank">aprantl@apple.com</a>> on behalf of Adrian Prantl <<a href="mailto:aprantl@apple.com" class="gmail_msg" target="_blank">aprantl@apple.com</a>><br class="gmail_msg">
<b class="gmail_msg">Date: </b>Thursday, February 23, 2017 at 10:52 AM<br class="gmail_msg">
<b class="gmail_msg">To: </b>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">Cc: </b>"<a href="mailto:reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org" class="gmail_msg" target="_blank">reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org</a>" <<a href="mailto:reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org" class="gmail_msg" target="_blank">reviews+D29833+public+d0b46dd83e56e03f@reviews.llvm.org</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>>, "<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>>, 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 class="gmail_msg">
<p class="MsoNormal gmail_msg"><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">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg">On Feb 23, 2017, at 10:08 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<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" style="margin-bottom:12.0pt"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">On Thu, Feb 23, 2017 at 9:35 AM Adrian Prantl <<a href="mailto:aprantl@apple.com" class="gmail_msg" target="_blank">aprantl@apple.com</a>> wrote:<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">On Feb 23, 2017, at 9:13 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">You were suggesting that the nice thing about the former was that the user couldn't fail to have a debug location:<br class="gmail_msg">
<br class="gmail_msg">
<span class="m_7649363730575125755gmailmsg gmail_msg"><span style="color:#212121" class="gmail_msg">"The primary benefit of having the API on Instruction is that we can make it impossible to accidentally create a locationless call instruction in a function with debuginfo (which would crash/assert the backend
when creating inline scopes)."</span></span><span style="color:#212121" class="gmail_msg"><br class="gmail_msg">
</span><br class="gmail_msg">
Which I'm a bit fuzzy on - if a new call instruction is created it'd still have no location so I'm not sure this choice of API provides extra defense against that mistake.<br class="gmail_msg">
<br class="gmail_msg">
If, today, an existing call instruction is moved then the location zero'd out - it's going to need work to replace the zeroing with merging no matter whether the API for that merging is on DebugLoc or Instruction.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</blockquote>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">Agreed.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg" style="margin-bottom:12.0pt"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><br class="gmail_msg">
So I guess coming back to your point: could you explain in more detail how having the API on Instruction provides a stronger guarantee/safety than having it elsewhere? I'm not clear on what that's defending against.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</blockquote>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">I want to make it harder to accidentally assign a null DebugLoc to a call. Currently getMergedDebugLoc() returns a null location (to save space in the line table) and at
each call site we have to write extra code to create a line:0 location if the instruction we are creating the merged location for is a call. See <a href="https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D29833&d=DwMFaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=kOsLCgQzH7N8ptZ7diJD9g&m=O6D-OeqGm2sNUSI30Qqz7sFjmurpqgcX9CVy6OgkNGw&s=_XNi_qHqSnfqrjV9rLI5uU6TiIsFhrAykuq40uy9pJQ&e=" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29833</a>, lib/Transforms/Utils/SimplifyCFG.cpp:1280
for an example — though upon closer inspection the original code was just keeping the original location of the call instead of assigning a line:0 location.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">I though it would be nice to hide this detail in the API, but this really makes it look like we want to decide how to handle call instructions (assign line:0, keep original
loc) on a case-by-case basis.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
</blockquote>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">Oh, right, sorry - sure, seems reasonable to make it part of Instruction so it can Do The Right Thing depending on whether the Instruction is a Call. (maybe a "setMergedLocation(DebugLoc,
DebugLoc)" that ignores the current location - I suppose in theory it could be a variadic function to handle more than two locations, but that probably doesn't come up often enough to worry? Not sure))<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
</div>
</blockquote>
<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">Yes, but then I realized that it is not obvious what The Right Thing is. When the instruction is a call, should we return a line:0 location, should we keep the original location? Is there one answer that fits all use-cases? I’m afraid now
that we would be hiding the fact that the user needs to do make an informed decision here behind the new API.<u class="gmail_msg"></u><u class="gmail_msg"></u></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">You mentioned abandoning this in favor of the other change under review? Seems the discussion is here so we might as well continue it here.<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
</div>
</blockquote>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
</div>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg">Sure, works for me.<u class="gmail_msg"></u><u class="gmail_msg"></u></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg">-- adrian<u class="gmail_msg"></u><u class="gmail_msg"></u></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg" style="margin-bottom:12.0pt"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">- Dave<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"> <u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<blockquote style="border:none;border-left:solid #cccccc 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0in;margin-bottom:5.0pt" class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg"><u class="gmail_msg"></u> <u class="gmail_msg"></u></span></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">thanks,<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
<div class="gmail_msg">
<p class="MsoNormal gmail_msg"><span style="font-size:9.0pt;font-family:"Helvetica","sans-serif"" class="gmail_msg">adrian<u class="gmail_msg"></u><u class="gmail_msg"></u></span></p>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal gmail_msg" style="margin-bottom:12.0pt"><u class="gmail_msg"></u> <u class="gmail_msg"></u></p>
</div></div></div></blockquote></div></div>