<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 22, 2017, at 4:25 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Feb 22, 2017 at 4:21 PM Adrian Prantl via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="">reviews@reviews.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-style: solid; border-left-color: rgb(204, 204, 204); padding-left: 1ex;">aprantl added a comment.<br class="gmail_msg"><br class="gmail_msg">[apologies for iterating so slowly at the moment]<br class="gmail_msg"><br class="gmail_msg">In<span class="Apple-converted-space"> </span><a href="https://reviews.llvm.org/D29833#682595" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29833#682595</a>, @twoh wrote:<br class="gmail_msg"><br class="gmail_msg">> Personally I prefer the existing API, because it doesn't allow the transient state before merging. For example, in lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp in this patch, DebugLoc of NewSI is not actually valid after line 1430. Existing API doesn't have this problem.<br class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg">If that is the only concern, would an additional convenience function solve (= sufficiently hide) the problem?<br class="gmail_msg"><br class="gmail_msg"> <span class="Apple-converted-space"> </span>Instruction::setMergedDebugLoc(DebugLoc A, DebugLoc B) {<br class="gmail_msg"> <span class="Apple-converted-space"> </span>setDebugLoc(A);<br class="gmail_msg"> <span class="Apple-converted-space"> </span>mergeDebugLocWith(B);<br class="gmail_msg"> <span class="Apple-converted-space"> </span>}<br class="gmail_msg"><br 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).<br class="gmail_msg"></blockquote><div class=""><br class=""></div><div class="">Either way we have a bug - either the profile will be buggy or the inlining broken, honestly since the buggy profile is harder to diagnose (because it has a location, but an incorrect one) I'd sort of favor an API that makes those bugs easy/the default, rather than the subtlety of incorrect locations.</div></div></div></div></blockquote><br class=""></div><div>If we have to make a choice between buggy profile and a broken inline info (that will assert) I agree with you. But can you remind me why the DILocation::getMergedDebugLocation() API prevents the buggy profile?</div><div><br class=""></div><div>-- adrian</div></body></html>