[Lldb-commits] [PATCH] D54747: Discard debuginfo for object files empty after GC
Rui Ueyama via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Mon Apr 8 23:36:35 PDT 2019
ruiu 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<InputSection>(Sec) || isa<MergeInputSection>(Sec))
----------------
ruiu wrote:
> rocallahan wrote:
> > rocallahan wrote:
> > > ruiu wrote:
> > > > ruiu wrote:
> > > > > S is true only when it is an InputSection, so you have duplicate tests for this. It also looks like you don't have to care about whether a section is an input section or a merged section. Isn't this condition sufficient?
> > > > >
> > > > > if (Sec->File && !IsLSDA)
> > > > > Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
> > > > Move this code after `Sec->Live = true` so that when we visit the same section more than once, this piece of code runs only once.
> > > We can't do that. The comment I added tries to explain why. Is it unclear?
> > I want to skip `EhInputSection` and `SyntheticSection` here. So I guess the advice to use `isa` here was wrong and I should revert to checking `kind() == Regular || kind() == Merge`?
> But you don't really care even if it is `EhInputSection` or `SyntheticSection`, no? I mean an assigned value would not be used but doesn't harm.
Ah, OK, got it.
================
Comment at: lld/ELF/MarkLive.cpp:190-191
+ // case we still need to mark the file.
+ if (S && !IsLSDA && Sec->File)
+ if (isa<InputSection>(Sec) || isa<MergeInputSection>(Sec))
+ Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
----------------
rocallahan wrote:
> rocallahan wrote:
> > ruiu wrote:
> > > ruiu wrote:
> > > > S is true only when it is an InputSection, so you have duplicate tests for this. It also looks like you don't have to care about whether a section is an input section or a merged section. Isn't this condition sufficient?
> > > >
> > > > if (Sec->File && !IsLSDA)
> > > > Sec->getFile<ELFT>()->HasLiveCodeOrData = true;
> > > Move this code after `Sec->Live = true` so that when we visit the same section more than once, this piece of code runs only once.
> > We can't do that. The comment I added tries to explain why. Is it unclear?
> I want to skip `EhInputSection` and `SyntheticSection` here. So I guess the advice to use `isa` here was wrong and I should revert to checking `kind() == Regular || kind() == Merge`?
But you don't really care even if it is `EhInputSection` or `SyntheticSection`, no? I mean an assigned value would not be used but doesn't harm.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54747/new/
https://reviews.llvm.org/D54747
More information about the lldb-commits
mailing list