[PATCH] D33614: Cloning: Fix debug info cloning

[David Blaikie via llvm-commits]

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

> GorNishanov added a comment.
>
> In https://reviews.llvm.org/D33614#766379, @dblaikie wrote:
>
> > The assertion is that, in the presence of a separate declaration (eg: a
> member function that is a coroutine - so the member variable gets a
> declaration (this is about the only time LLVM has/uses a separate
> declaration for a function/subprogram in the debug info) and models an out
> of line definition (even if the definition is in-line)) that if there is a
> linkage name for the declaration, then it's the same as the linkage name
> for the definition.
> >
> > This seems pretty wrong for coroutines where the debug info looks like
> it's going to have 3-4 functions, with different mangled names, all that
> claim to be an implementation of the same declaration? Their mangled names
> won't match the declaration anyway...
> >
> > I think maybe, it'd be better to drop the linkage name when cloning. (or
> use the linkage name of the clone - and change/relax the assertion) It
> won't be better in the end - the debug info would still have multiple
> definitions referring to the same declaration and if the declaration has a
> linkage name it'll be right for at most one of those functions and wrong
> for the rest. But at least the IR won't be /as/ wrong... ish.
>
>
> Something like this?
>
> Replace:
>
>   SP->getName(), NewFunc->getName()  (rL302576 way)
>
> or
>
>   SP->getName(), SP->getLinkageName() (current patch way)
>
> with
>
>   NewFunc->getName(), NewFunc->getName(),  (dblakie way?)
>

This won't work either - because the new subprogram is still referring to
the original subprogram's declaration which has the original linkage name -
so they'll conflict, etc.

What I was suggesting was likely passing no linkage name:

  SP->getName(), "", ...

That way it won't conflict, at least, I think. But it may be useful to use
the NewFunc->getName() as the name (the first of the two parameters) too.
But I'd leave that for a separate commit with due consideration to what the
debugging/debuginfo behavior should be. It shouldn't cause
crashes/asserts/etc generating that debug info, at least.

- Dave


>
>
> https://reviews.llvm.org/D33614
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170527/6b6f6e1c/attachment.html>