[PATCH] WIP: AddDiscriminators: Check more carefully for equivalent locations

[Duncan P. N. Exon Smith]

> On 2015-Apr-10, at 10:40, Diego Novillo <dnovillo at google.com> wrote:
> 
> On Fri, Apr 10, 2015 at 1:33 PM, Duncan P. N. Exon Smith
> <dexonsmith at apple.com> wrote:
>> 
>>> On 2015-Apr-10, at 10:24, David Blaikie <dblaikie at gmail.com> wrote:
>>> 
>>> 
>>> 
>>> On Thu, Apr 9, 2015 at 5:26 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>> Stop using `DILocation::atSameLineAs()` to check whether two locations
>>> are equivalent.  This check is too weak: it ignores any column info that
>>> might be present, and ignores the scope/inlined-at chains.
>>> 
>>> I don't think we can use the scope/inlined-at can be used here (but, as Eric said on IRC, we might not have testing for it) - we use the scope information to build the scope hierarchy in the debug_info, but it doesn't affect the line table at all. So we could have two debug locations with the same line/file/column but different scopes, yet the same basic block (something as simple as {} can introduce a scope without introducing a new basic block, of course, or a do-while loop would still have a contiguous range but be two basic blocks) and thus they'd coalesce to the same line table entry when they should be separate blocks, potentially.
>>> 
>>> Does that make sense?
>> 
>> Yup, makes sense.  I think where I went wrong was that currently we
>> *do* get different line table entries (sounds like that's a bug/missed
>> optimization?).
>> 
>> But using column info makes sense, right?  I'll write a FIXME and file
>> a PR for that (sounds like maybe you guys need to update tools on your
>> end?).
> 
> There is an additional tooling problem here, yes.  I don't think that
> Linux Perf uses column information when emitting the profile, so we
> cannot retrieve them when converting them into LLVM's profile.
> 
> If you are removing this test to rely more on column numbers, I think
> you'll break users of Linux Perf. Too bad we can't really test in
> tree. But, if you must get rid of this, I can look at what needs to be
> fixed in the tools outside of LLVM.

That'd be great.  I'm not in a particular hurry (although maybe someone
else is?); I'm content with a FIXME and a PR in the short-term.  I'll
CC you on the PR when I file it!