[PATCH] D29833: Improve the API of DILocation::getMergedLocation()

[David Blaikie via llvm-commits]

You were suggesting that the nice thing about the former was that the user
couldn't fail to have a debug location:

"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)."

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.

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.

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.

On Thu, Feb 23, 2017 at 9:09 AM Adrian Prantl <aprantl at apple.com> wrote:

>
> > On Feb 22, 2017, at 4:34 PM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > If an instruction is hoisted across a BB without adjusting the location,
> a sample based profile taken at that location would incorrectly count a hit
> to the source BB even though it may not be executed. (eg: CSE moves similar
> code from an 'if' and 'else' block to before the predecessor (or sinks it
> to a successor) and picks the 'if' block's instructions as the ones to move
> (deleting the 'else' block) - now every time the profile observes the
> evaluation of the common subexpression it counts it as the 'if' block
> executing - when it's possible the 'if' block never executes and only the
> 'else' block is ever chosen)
>
> That part makes sense to me, my question was more how this would affect
> our choice between:
>
>   Instruction::mergeLocationWith(DebugLoc Other);
>   Instruction::setMergedLocation(DebugLoc A, DebugLoc B);
>
> and
>
>   static DebugLoc::getMergedLocation(DebugLoc A, DebugLocB);
>
> ?
>
> -- adrian
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170223/6bf93962/attachment-0001.html>