[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