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<InputSection>(Sec) || isa<MergeInputSection>(Sec))
----------------
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?
================
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:
> 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`?
================
Comment at: lld/ELF/MarkLive.cpp:313
// remove its relocation section.
+ bool AnyDebugSections = false;
for (InputSectionBase *Sec : InputSections) {
----------------
ruiu wrote:
> Let's remove this optimization -- I don't think we need this.
OK
================
Comment at: lld/ELF/MarkLive.cpp:321
bool IsLinkOrder = (Sec->Flags & SHF_LINK_ORDER);
bool IsRel = (Sec->Type == SHT_REL || Sec->Type == SHT_RELA);
----------------
ruiu wrote:
> Then you can simplify the condition to
>
> if (!IsAlloc && !IsLinkOrder && !IsRel && !Sec->Debug)
> Sec->Live = true;
Yep.
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
