[PATCH] D29170: [ELF] - Remove unnessesary checks from GC logic.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 27 00:14:08 PST 2017


grimar added a comment.

> In https://reviews.llvm.org/D29170#657896, @ruiu wrote:
> 
>> I don't know if this is making things better - it seems neutral to me or an unnecessary churn.

Interesting. Is not it the same what you mentioned during review of https://reviews.llvm.org/D28612 ?

  "So, the right way of doing it is to set Live bit in the ctor if it should never be GC'ed."

Instead of checking the sections flags everywhere patch relies on Live bit now.

> Same. I'm actually a bit confused about this refactoring. First we had the logic in a single place and now we moved part of it to `InputSection.{cpp,h}` (i.e. the assertion became an if).

https://reviews.llvm.org/D28612 adds SHT_RELA and SHT_REL sections that should be involved in GC (because .rela.text depends on .text and we do not want to keep REL[A] sections unconditionally). 
REL[A] sections are not SHF_ALLOC, that means I need to set up GC logic for them.

With this patch only I need is to add that to constructor of InputSectionData which sets the Live bit now:

  !Config->GcSections || !(Flags & SHF_ALLOC)), //Add here

Patch changes conditions of GC to use Live bit instead of rechecking the sections flags again.

Next part you are talking about is different:

  void markLiveAt(uintX_t Offset) {
    if (this->Flags & SHF_ALLOC)
      LiveOffsets.insert(Offset);
  }

It is a member of MergeInputSection, and MergeInputSection has multiple same checks in it's methods because its behavior is different for allocatable and nonallocatable
cases. So this part of patch localizes that check and isolates inside MergeInputSection instead of spreading it to GC code.


https://reviews.llvm.org/D29170





More information about the llvm-commits mailing list