[PATCH] D33614: Cloning: Fix debug info cloning

[David Blaikie via llvm-commits]

distinct nodes are a bit more expensive (due to the generalized
deduplication) so probably better to not rely on them. Alternatively, if
you are relying on tehm - why not rely on them solely, rather than usuing
the cache as well? Seems weird to have a hybrid approach.

On Fri, May 26, 2017 at 9:24 PM Gor Nishanov via Phabricator <
reviews at reviews.llvm.org> wrote:

> GorNishanov added inline comments.
>
>
> ================
> Comment at: lib/IR/DebugLoc.cpp:170
> +
> +    auto *NewVar = DILocalVariable::get(
> +      Ctx,
> ----------------
> GorNishanov wrote:
> > dblaikie wrote:
> > > Is there a reason this can't be distinct anymore?
> > >
> > > You mentioned in the description: "I added caching, but, it is not
> sufficient, since dbg.value may refer to various expressions representing
> the same user var, thus, switching form getDistinct to get is important."
> > >
> > > But I don't follow quite what you're saying there - could you explain
> it in more detail/in another way?
> > >
> > > What happens without changing this to 'get'?
> > It can still creates duplicate DILocalVariables. I have it in the very
> large test case, I can see if I can reduce it to something manangable.
> Thank you for looking at this!
> Hah. Cache by itself does not work since it is cleared for every basic
> block.
> I am not sure what are the implications of moving it outside of
> CloneBasicBlock.
>
> I would keep the cache where it is, and keep get as opposed to getDistinct
> to collapse duplicate LocalVariables to the same one.
>
>
> https://reviews.llvm.org/D33614
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170527/4545f6b8/attachment.html>