[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC

[Eric Christopher via lldb-commits]

For now I think the compelling argument on revert is:

- this breaks currently working thinlto builds

whether or not this has uncovered a latent problem in thinlto, the
linking strategy used, or something else I think we should revert
until we can take a look and decide where the problems are and what
should be done about them.

Thanks

-eric

On Thu, May 16, 2019 at 4:17 PM Bob Haarman via Phabricator
<reviews at reviews.llvm.org> wrote:
>
> inglorion added a comment.
>
> I also think we should revert this change while we think about the approach. I've been reluctant to do this because I was impressed by how many bytes of output it saves, and I didn't want to lose that just because it doesn't play nice with ThinLTO. However, after some more data gathering and talking to people, I think that:
>
> 1. There is enough doubt that this is the right approach that we probably should take some time to think about which way we really want to go.
>
> 2. In the meantime, a build configuration that used to work is broken.
>
> 3. To fix that while leaving this change in, we would have to build more things on top of the change, which seems unwise if we're not sure that this is the best approach.
>
> So I think we should revert and rethink.
>
> Reasons I'm not convinced that this is the right approach:
>
> 1. We have now identified multiple use cases that break with this change. ThinLTO is one, -fmodules-debuginfo is another. The general theme here is that the expectation is that if an object file is passed to the linker and not between --start-lib and --end-lib or in a static library, the expectation is that it makes it into the final link. -gc-sections asks to discard unused sections, but it's not clear that this should also apply to debug information, and we have seen some problems.
>
> 2. It's also not clear to me that this is as much of a win as I initially thought. It is in the cases that have been demonstrated, of course, but I'm not sure how well that generalizes. On a local Clang build I performed, for example, it made exactly 0 bytes difference. Perhaps the big wins only happen in pathological cases that are better addressed some other way.
>
> So I'm going to revert this to unbreak the Chromium/Android build, and then we can think some more about how we can get the size win without the breakage.
>
>
> Repository:
>   rL LLVM
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D54747/new/
>
> https://reviews.llvm.org/D54747
>
>
>