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

[Rui Ueyama via Phabricator via lldb-commits]

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:
> > > > 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`?
> > This needs a comment.
> > 
> > Do you have to visit each file during the mark phase? Looks like you can mark only sections first, and after marking all sections, you can scan all sections to mark files. Looks like they can be two separate stages.
> Maybe I'm wrong but I would have expected adding another pass over all sections to be slower than what I'm doing here.
> 
> Also we need to distinguish LSDA sections from other live sections since LSDA does not count as "making the file live". So we'd have to add an LSDA flag to `InputSection`.
> 
> Are you sure you want me to make this change?
> 
> Also I just noticed I've written LDSA in several places where it should be LSDA!
Ah OK, I thought that you set Sec->Live  to true only in `setSectionLive()` but you manipulated that in EnqueueMaybeLDSA as well. Looks like we have too many callback functions in this function -- this file is organized that way because the callback functions were very simple. Now it's been growing organically and probably get to the point that we should just use the regular class-based abstraction. Let me do that first.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D54747