[PATCH] D65909: ELF: Move sections referred to by __start_/__stop_ symbols into the main partition.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/MarkLive.cpp:299
 template <class ELFT> void MarkLive<ELFT>::moveToMain() {
-  for (InputFile *file : objectFiles)
-    for (Symbol *s : file->getSymbols())
+  StringSet<> startStopSymbolNames;
+  for (InputFile *file : objectFiles) {
----------------
MaskRay wrote:
> ruiu wrote:
> > MaskRay wrote:
> > > `DenseSet<StringRef>` can avoid allocation for strings.
> > In this patch you scan over all input files and their symbol tables to see if there's a symbol starting with "__start_" or "__stop_", but do you have to do this that way? I mean
> > 
> > 1. most section names are not a valid C identifier in the first place,
> > 2. if section whose name "foo" exists, look up the symbol table with "__start_foo" and "__end_foo" to see if there's a symbol that refer to the start/end of the sectoin
> > 
> > Am I missing somehting?
> The 
> 
> ```
> for (InputFile *file : objectFiles)
>   for (Symbol *s : file->getSymbols())
> ```
> 
> loop already exists because STT_GNU_IFUNC and STT_TLS are special cased. The loop gets reused to check `__start_` and `__stop_`. I am not sure an alternative approach will optimize it.
Yeah, performance-wise I don't know which is better, but I feel like looking up the symbol table is more natural than scanning all symbols if we are handling symbol names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65909