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

Duncan P. N. Exon Smith dexonsmith at apple.com
Mon Jan 19 10:50:26 PST 2015


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



More information about the llvm-commits mailing list