[PATCH] D55309: ThinLTO: Do not import debug info for imported global constants

[David Blaikie via llvm-commits]

On Wed, Dec 5, 2018 at 11:37 AM Teresa Johnson via Phabricator <
reviews at reviews.llvm.org> wrote:

> tejohnson accepted this revision.
> tejohnson added a comment.
> This revision is now accepted and ready to land.
>
> LGTM to address regression, especially since the debug info already isn't
> maintained for global variables that are const propagated. David, as a
> follow on it would be good to understand if/when there are cases where the
> debug info should be imported for these variables (i.e. if they aren't in
> fact constant propagated after internalization).
>

Sure - would you prefer that in the form of an email discussion,
documentation, further elaborated comments (might get a bit verbose, but I
did try to cover some of the tricks there)?

(as an aside: We did some tricks to ensure that ThinLTO+Split DWARF only
produces a single CU (for a few reasons - Split DWARF wasn't really well
defined with multiple CUs, GDB couldn't cope with it, and especially it
didn't have any well defined way of doing cross-CU references to describe
the importing anyway), but looking at how much this regression grew debug
info - I wonder if anyone (even those not using Split DWARF) would really
want all the little CU fragments to be emitted as separate CUs in each
ThinLTO object file - it looks not great, even if it is somewhat more
accurate to the source code (as best it can in the medium term at least) -
DWARF isn't really built for all these tiny CUs - though if Apple folks
haven't been complaining about the debug info size for their ThinLTO use...
maybe that's good enough?)


>
>
>
> ================
> Comment at: lib/Linker/IRMover.cpp:1065
>      ValueMap.MD()[CU->getRawRetainedTypes()].reset(nullptr);
> +    ValueMap.MD()[CU->getRawGlobalVariables()].reset(nullptr);
>
> ----------------
> Please add a comment here (the old one that was removed isn't quite
> accurate now, but could probably be modified).
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D55309/new/
>
> https://reviews.llvm.org/D55309
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181205/5b5e147e/attachment.html>