[PATCH] Ensure debug info for two calls to the same function from the same location are not merged

[David Blaikie]

On Mon, Jan 19, 2015 at 10:50 AM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-Jan-19, at 10:28, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Fri, Dec 19, 2014 at 5:13 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >
> > > On 2014-Dec-19, at 15:10, David Blaikie <dblaikie at gmail.com> wrote:
> > >
> > >
> > >
> > > On Wed, Dec 3, 2014 at 2:18 PM, David Blaikie <dblaikie at gmail.com>
> wrote:
> > >
> > >
> > > On Wed, Dec 3, 2014 at 2:08 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > >
> > > > On 2014-Dec-03, at 13:12, David Blaikie <dblaikie at gmail.com> wrote:
> > > >
> > > >
> > > >
> > > > On Thu, Nov 27, 2014 at 9:41 AM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > > >> Disabling uniquing for inlined locations seems like a pretty
> > > >> heavy hammer to me, and I worry that in contexts with heavy
> inlining it
> > > >> could cause an explosion.
> > > >>
> > > > Any case where this creates more nodes due to inlining are cases
> that are buggy currently, so far as I know.
> > > >
> > > > The point of adding the column info was to make every call site
> distinct from every other call site - this uniquing just does the same
> thing but entirely consistently, as far as I understand it.
> > >
> > > Ah, okay.  This makes sense to me now.
> > >
> > > I made a few testcases of my own and ran with/without your patch and I
> > > better understand what you've done.  I'd imagined a bunch of
> interactions
> > > with `DebugLoc` that your approach avoids.
> > >
> > > >> Assuming disabling uniquing is the right goal, I really don't like
> this
> > > >> self-reference pattern.  I'm hoping to have a patch out early next
> week
> > > >> (I've been on holiday this week) that splits metadata and value --
> and
> > > >> one of the first follow-ups will be a change to MDNode that allows
> > > >> "opting-out" of uniquing.  Do you think it makes sense to wait and
> use
> > > >> that, rather than introducing and pulling out an extra field?
> > > >
> > > > Another reason I sent this for review - I don't mind much
> waiting/how it gets done, just that it gets done.
> > >
> > > Let's wait then.  I'm hoping to get things moving along soon.
> > >
> > > Sure thing.
> > >
> > > Ping on this - but expect it'll be some time in the new year,
> >
> > Leaving in 2 minutes for some holiday :).
> >
> > > so I'll try to remember to check back in a month or so.
> >
> > Please do!
> >
> > Pinging just so I don't forget.
> >
> > Now that we have 'distinct', I guess I just need to change the stuff
> that builds 'inlinedAt' location nodes to use that flag, perhaps?
>
> Yeah, I think calling `MDLocation::getDistinct()` just does what you
> want here.


Yep, seems to work basically the same. Committed in r226736.

Thanks!
- David
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150121/9e67ec4b/attachment.html>