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

[Robert O'Callahan via Phabricator via llvm-commits]

rocallahan marked 3 inline comments as done.
rocallahan added a comment.

> @rocallahan I find that people are discussing a generic approach in D59553 <https://reviews.llvm.org/D59553>

Thanks for the reference. It seems to me that there is no consensus there yet for any approach that would obsolete this change. Of the three approaches discussed there:

1. Having the linker rewrite DWARF: seems to have been decisively rejected.
2. Give each function standalone debuginfo in COMDAT: sounds to me like it would be very inefficient, both in the size of intermediate files and in the size of the final binary. DWARF type information currently shared between functions in the same CU would have to be duplicated, and eliminating that duplication would require the linker rewrite DWARF, which brings us back to point 1.
3. Optimize away unnecessary debuginfo using a post-link tool like `dwz`: Considerably less efficient than not writing out the bloated DWARF in the first place.



================
Comment at: lld/ELF/MarkLive.cpp:192
+  Sec->Live = true;
+  if (Sec->kind() != SectionBase::Kind::Regular &&
+      Sec->kind() != SectionBase::Kind::Merge)
----------------
MaskRay wrote:
> This check can be changed to `!isa<InputSection> && !isa<MergeInputSection>`. But do you just want to exclude `EhInputSection`?
Shouldn't I also be excluding `SyntheticSection`?


================
Comment at: lld/ELF/MarkLive.cpp:195
+    return;
+  if (!Sec->File || !ObjFile<ELFT>::classof(Sec->File))
+    return;
----------------
MaskRay wrote:
> > `!ObjFile<ELFT>::classof(Sec->File)`
> 
> Can this happen?
I think not. Happy to remove this check if you don't want it.


================
Comment at: lld/ELF/MarkLive.cpp:199
+    Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
+  }
+}
----------------
MaskRay wrote:
> The brace is redundant.
Thanks.


Repository:
  rLLD LLVM Linker

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D54747/new/

https://reviews.llvm.org/D54747