[PATCH] D71157: [ELF] Refine section group --gc-sections rules to not discard .debug_types

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:630
+        hasAlloc = true;
+    }
+
----------------
grimar wrote:
> I'd suggest adding a helper, because `initializeSections` becoming too large probably.
> Something like:
> 
> 
> ```
> template <typename ELFT>
> static std::vector<InputSectionBase *>
> getGroupSections(ArrayRef<typename ELFT::Word> entries,
>                  ArrayRef<InputSectionBase *> sections) {
>   std::vector<InputSectionBase *> ret;
>   for (uint32_t i : entries)
>     if (i < sections.size())
>       if (InputSectionBase *s = sections[i])
>         if (s != &InputSection::discarded)
>           ret.push_back(s);
>   return ret;
> }
> ```
> 
> Then you can simplify a bit:
> 
> ```
>   for (ArrayRef<Elf_Word> entries : selectedGroups) {
>     bool hasAlloc = false;
>     std::vector<InputSectionBase *> sections =
>         getGroupSections<ELFT>(entries.slice(1), this->sections);
>     for (InputSectionBase *s : sections) {
>       if ((s->flags & SHF_ALLOC) == 0)
>         continue;
>       hasAlloc = true;
>       break;
>     }
> 
>     // If no member has the SHF_ALLOC flag, retain the whole group. This rule
>     // retains .debug_types and .rela.debug_types.
>     if (!hasAlloc)
>       continue;
> 
>     InputSectionBase *head;
>     InputSectionBase *prev = nullptr;
>     for (InputSectionBase *s : sections) {
>       if (prev)
>         prev->nextInSectionGroup = s;
>       else
>         head = s;
>       prev = s;
>     }
>     if (prev)
>       prev->nextInSectionGroup = head;
>   }
> ```
> 
> I'd probably consider moving out the whole code related to groups into it's own method/helper though.
> (not in this patch)
I will move out the whole section group code. The split-off function is not too long, so we don't need the helper function (getGroupSections).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71157