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<InputSection>(Sec) || isa<MergeInputSection>(Sec))
----------------
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.
================
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;
----------------
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;
================
Comment at: lld/ELF/MarkLive.cpp:313
// remove its relocation section.
+ bool AnyDebugSections = false;
for (InputSectionBase *Sec : InputSections) {
----------------
Let's remove this optimization -- I don't think we need this.
================
Comment at: lld/ELF/MarkLive.cpp:321
bool IsLinkOrder = (Sec->Flags & SHF_LINK_ORDER);
bool IsRel = (Sec->Type == SHT_REL || Sec->Type == SHT_RELA);
----------------
Then you can simplify the condition to
if (!IsAlloc && !IsLinkOrder && !IsRel && !Sec->Debug)
Sec->Live = true;
================
Comment at: lld/ELF/MarkLive.cpp:330
+ if (AnyDebugSections)
+ // Mark debug sections as live in any object file that has a live
----------------
Remove this variable and just visit all sections even if there's no debug info
(which shouldn't take too much time anyway.)
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
