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

> > Hello, in a bit of a https://xkcd.com/1172/ moment this breaks the > chromium/android build. We have a list of "resources" (strings, bitmaps, > etc) that we list in an XML file which then generates a header with lots of > "IDR_foo" constants. As it turns out, now all of these resources are

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

rocallahan added a comment. Sorry for not responding earlier. Phabricator insists on sending me emails about all issues in the system, and I never figured out how to get GMail to show me only emails about Phabricator issues I'm CCed on. > I know little about Rust crates but I worry some

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

thakis added a comment. For posterity, https://bugs.chromium.org/p/chromium/issues/detail?id=960881#c31 has the explanation for what exactly was happening in the Chromium/Android build. It's a bit different from rnk's example since no cross-TU imports are happening. It likely explains why rnk

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

ruiu added a comment. Bob, thank you for reverting this. So, Robert, looks like this idea didn't work well. We need a different solution. And perhaps a better approach is to use --start-lib and --end-lib. You found that these options didn't work well for your input, but I don't fully

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

thakis added a comment. echristo's comments on the mail thread for this didn't make it to phab. I apologize for repeating more or less everything he said, I had looked on phab before reading email. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/

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

inglorion added a comment. Reverted in r360955. 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

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

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

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

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

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

Unlikely. I'm in the middle of getting some, but that's still going to be a few months I think. And yes, we should probably just revert for now. I can unless someone beats me to it. Thanks! On Thu, May 16, 2019 at 2:29 PM Nico Weber via Phabricator wrote: > > thakis added a comment. > > This

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

thakis added a comment. This breaking both debug info in thinlto builds and fmodules-debuginfo is probably enough to revert and go back to the drawing board. I suppose we don't have any debug info quality tests that use thinlto? Repository: rL LLVM CHANGES SINCE LAST ACTION

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

rnk added a comment. There's another use case worth considering here, which is -fmodules-debuginfo. In that situation, a standalone object file is emitted that contains nothing but DWARF .debug_info sections, and the object is fed to the linker. If the linker drops it with --gc-sections, it's

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

MaskRay added a comment. In D54747#1459175 , @rocallahan wrote: > Updated results for the rusoto test in > https://github.com/rust-lang/rust/issues/56068#issue-382175735. The test > changed a bit because I'm using an updated Rust toolchain and

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

rnk added a comment. I think there's an issue with the whole idea of dropping .debug_info from objects without live sections. Consider: // a.cpp int main() { return f(); } // b.cpp struct Foo { int x, y; }; int f() { volatile Foo var; var.x = 13; var.y = 42;

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

Tagging in Teresa as well for the thinlto parts. On Wed, May 8, 2019 at 12:43 PM Nico Weber via Phabricator wrote: > > thakis added a comment. > > That problem only seems to happen when (thin) lto is enabled. > > > Repository: > rL LLVM > > CHANGES SINCE LAST ACTION >

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

thakis added a comment. That problem only seems to happen when (thin) lto is enabled. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747 ___ lldb-commits mailing list

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

thakis added a comment. Hello, in a bit of a https://xkcd.com/1172/ moment this breaks the chromium/android build. We have a list of "resources" (strings, bitmaps, etc) that we list in an XML file which then generates a header with lots of "IDR_foo" constants. As it turns out, now all of these

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

This revision was automatically updated to reflect the committed changes. Closed by commit rL358069: Discard debuginfo for object files empty after GC (authored by ruiu, committed by ). Changed prior to commit: https://reviews.llvm.org/D54747?vs=194255=194469#toc Repository: rL LLVM

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

ruiu added a comment. I will do that. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747 ___ lldb-commits mailing list lldb-commits@lists.llvm.org

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

ruiu accepted this revision. ruiu added a comment. This revision is now accepted and ready to land. LGTM I'd write a comment explaining why we are handling debug sections in a special way, but that can be done later. Please submit. Thank you for doing this! CHANGES SINCE LAST ACTION

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

rocallahan marked an inline comment as done. rocallahan added inline comments. Comment at: lld/ELF/MarkLive.cpp:190-191 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) + Sec->getFile()->HasLiveCodeOrData = true;

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

ruiu added inline comments. Comment at: lld/ELF/MarkLive.cpp:190-191 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) + Sec->getFile()->HasLiveCodeOrData = true; rocallahan wrote: > ruiu wrote: > >

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

rocallahan updated this revision to Diff 194255. rocallahan edited the summary of this revision. rocallahan added a comment. The results are basically unchanged with the section type checks removed, so I've just gone ahead and done that. CHANGES SINCE LAST ACTION

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

rocallahan marked 4 inline comments as done. rocallahan added inline comments. Comment at: lld/ELF/MarkLive.cpp:190 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) ruiu wrote: > ruiu wrote: > > S is

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

ruiu added a comment. Overall looking good. Comment at: lld/ELF/MarkLive.cpp:190 + // case we still need to mark the file. + if (S && !IsLSDA && Sec->File) +if (isa(Sec) || isa(Sec)) ruiu wrote: > S is true only when it is an InputSection, so you have

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

ruiu added a comment. Nice. One more thing you might want to try is to add `-O2` to the linker command line option. When that option is given, lld attempts to tail-merge strings in the string table. That's not very effective, but you might be able to shave off 0.2% or something like that.

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

rocallahan added a comment. I am also pleased to report that for my real project , switching from `ld.bfd` to `lld` + this patch reduces the total size of our `dist` built binaries from 2.9GB to 2.0GB. I'm not sure why it's become

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

rocallahan added a comment. Updated results for the rusoto test in https://github.com/rust-lang/rust/issues/56068#issue-382175735. The test changed a bit because I'm using an updated Rust toolchain and `rusoto_core` 0.37.0. | Linker | Size (bytes) | Real time

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

rocallahan updated this revision to Diff 194244. rocallahan added a comment. Addressed all comments AFAIK. I'll post some performance numbers in a moment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747 Files: lld/ELF/Driver.cpp

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

ruiu added a comment. I committed https://reviews.llvm.org/D59800 which I believe makes your change easier to follow once rebased. Could you rebase it and upload a patch? Thanks! Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/

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

ruiu added inline comments. Comment at: lld/ELF/MarkLive.cpp:192 + Sec->Live = true; + if (Sec->kind() != SectionBase::Kind::Regular && + Sec->kind() != SectionBase::Kind::Merge) rocallahan wrote: > ruiu wrote: > > rocallahan wrote: > > > MaskRay wrote: >

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

rocallahan marked 2 inline comments as done. rocallahan added inline comments. Comment at: lld/ELF/MarkLive.cpp:190 +template static void setSectionLive(InputSectionBase *Sec) { + Sec->Live = true; ruiu wrote: > Since this file is MarkLive, markSection is

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

ruiu added inline comments. Comment at: lld/ELF/MarkLive.cpp:190 +template static void setSectionLive(InputSectionBase *Sec) { + Sec->Live = true; Since this file is MarkLive, markSection is perhaps a better name. Comment at:

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

rocallahan added a comment. Another clarification: > DWARF type information currently shared between functions in the same CU > would have to be duplicated. I guess that's not necessary. The compiler could put all debuginfo that's shared between functions into a non-COMDAT section for the

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

rocallahan added a comment. To be clear, I think the best long-term solution is for LLD to rewrite the DWARF, but from my (admittedly limited) perspective that seems to be at best a distant prospect. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION

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

rocallahan added a comment. > Do you have numbers with ld.bfd and gold? I don't have numbers for that exact test with those linkers but they basically behave the same way as LLD. Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/

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

rocallahan marked 3 inline comments as done. rocallahan added a comment. > @rocallahan I find that people are discussing a generic approach in D59553 > Thanks for the reference. It seems to me that there is no consensus there yet for any approach that would

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

MaskRay added a comment. @rocallahan I find that people are discussing a generic approach in D59553 Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747

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

MaskRay added a comment. In D54747#1342666 , @rocallahan wrote: > Here are some results for the rusoto test in > https://github.com/rust-lang/rust/issues/56068#issue-382175735: > > | LLD | Binary size in bytes | > | LLD 6.0.1

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

MaskRay added inline comments. Comment at: lld/ELF/MarkLive.cpp:192 + Sec->Live = true; + if (Sec->kind() != SectionBase::Kind::Regular && + Sec->kind() != SectionBase::Kind::Merge) This check can be changed to `!isa && !isa`. But do you just want to

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

rocallahan added a comment. Herald added a project: LLVM. Ping? Repository: rLLD LLVM Linker CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54747/new/ https://reviews.llvm.org/D54747 ___ lldb-commits mailing list

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

rocallahan added a comment. I agree that ideally the linker would be able to do fine-grained GC of DWARF at the granularity of individual DIEs and other data items. However, implementing that would be a huge project. Currently AFAICT `lld` does very little DWARF processing. Wholesale DWARF

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

rocallahan added a comment. Here are some results for the rusoto test in https://github.com/rust-lang/rust/issues/56068#issue-382175735: | LLD | Binary size in bytes | | LLD 6.0.1 | 43,791,192 | | LLD 8.0.0 1cfcf404f6b23943405bc3c5bb5940b10b914624

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

rocallahan added a comment. Sorry for the delayed reply. I just discovered that Phabricator nofications were being buried by my mail filters. In D54747#1312161 , @ruiu wrote: > What you are doing in this patch is not too complicated and makes sense to

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

ruiu added a comment. It seems to me that just adding `--start-lib` to his command line can fix the issue, so I'm waiting for Robert's response. If it doesn't work for some reason, we can analyze why it doesn't work and then discuss what we can do for his problem. Repository: rLLD LLVM

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

echristo added a comment. In D54747#1315241 , @ruiu wrote: > But 2GB is perhaps still too big and I guess a large part of it can be for > dead sections. If we fix this, I'd like to fix it in a proper way so that we > can completely eliminate debug info

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

ruiu added a comment. But 2GB is perhaps still too big and I guess a large part of it can be for dead sections. If we fix this, I'd like to fix it in a proper way so that we can completely eliminate debug info for dead sections. rocallahan, can I ask why Rust passes all object files to the

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

echristo added a comment. In D54747#1312161 , @ruiu wrote: > Thank you for the patch. > > What you are doing in this patch is not too complicated and makes sense to > me. That said, if actual size saving is not significant as you said in >

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

MaskRay added a comment. If we decide to optimize DWARF garbage collection, something generic will be cool. This generic-abi thread has some discussion about that https://groups.google.com/d/msg/generic-abi/A-1rbP8hFCA/EDA7Sf3KBwAJ (e.g. using COMDAT but it seems challenging and it comes with

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

ruiu added a comment. Thank you for the patch. What you are doing in this patch is not too complicated and makes sense to me. That said, if actual size saving is not significant as you said in https://github.com/rust-lang/rust/issues/56068#issuecomment-440160568, it may not be worth doing. It

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

rocallahan created this revision. rocallahan added a reviewer: ruiu. rocallahan added a project: lld. Herald added subscribers: llvm-commits, MaskRay, JDevlieghere, arichardson, emaste. Herald added a reviewer: espindola. Rust projects tend to link in all object files from all dependent