[PATCH] D25742: Remove debug location from common tail when tail-merging

[David Blaikie via llvm-commits]

That's what discriminators are for - but I've no idea where that fits in
the workflow etc.

In any case I'm not sure it's worth worrying about reusing the same line
when it happens to be all on the same line (& removing the column - what a
debugger would do seeing some things with column info and some without etc)

On Tue., 18 Oct. 2016, 4:27 pm Paul Robinson, <paul.robinson at sony.com>
wrote:

> probinson added a comment.
>
> In https://reviews.llvm.org/D25742#573687, @aprantl wrote:
>
> > In https://reviews.llvm.org/D25742#573401, @probinson wrote:
> >
> > > In https://reviews.llvm.org/D25742#573352, @aprantl wrote:
> > >
> > > > This approach seems generally fine, but I have one question:
> > > >
> > > > If the code were on a single line, and both locations share a common
> ancestor scope, it seems make sense to create a new location using the
> common ancestor scope and line and only remove the column information.
> > >
> > >
> > > That would collapse the if-then-else into (effectively) a single
> statement.  That probably works okay for a debugger but not profiling,
> which still wants to treat the then/else as distinct.  And, after the tail
> merging, the tails are no longer distinct.
> >
> >
> > I'm not sure I understand your point. How is having an orphaned add
> instruction preferable over having it associated with the collapsed
> if-then-else statement? Wouldn't I want that instruction to be counted
> towards the line?
>
>
> No, the profiler wants to assign sample counts to each block
> individually.  By giving each block the same source attribution, you assert
> that they have the same profiles.  That's unlikely to be true in practice.
> Really what happens is that the sample counts would be assigned to the
> parent block, which doesn't help the profiler sort out what to do with the
> nested blocks.
>
> Admittedly, zapping the source attribution on the merged (parts of the)
> blocks doesn't let you attribute those counts to *any* block, but at least
> you aren't attributing counts incorrectly.
>
> Robert may want to correct some of my assumptions here, but this is my
> understanding.
>
>
> https://reviews.llvm.org/D25742
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161018/b87c2cee/attachment.html>