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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

The logic looks OK to me. A suggestion about the code is inline.



================
Comment at: lld/ELF/InputFiles.cpp:618
 
   // For each secion group, connect its members in a circular doubly-linked list
   // via nextInSectionGroup. See the comment in markLive().
----------------
(while you are here)
secion -> section


================
Comment at: lld/ELF/InputFiles.cpp:630
+        hasAlloc = true;
+    }
+
----------------
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)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71157