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
<revi...@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
>
>
>
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to