[PATCH] D77807: [LLD][ELF] Implement --discard-* for cases when -r or --emit-relocs are used.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Thanks for the update. The member initializer of `used` which I was not comfortable is not removed. The code looks pretty good now.



================
Comment at: lld/ELF/Writer.cpp:650
+
+template <class ELFT> static void markUsedLocalSymbols() {
+  for (InputFile *file : objectFiles) {
----------------
Add a comment that when --gc-sections is not specified, this function is needed to set the `used` bits


================
Comment at: lld/ELF/Writer.cpp:671
 
-  // If --emit-reloc or -r is given, all symbols including local ones need to be
-  // copied because they may be referenced by relocations.
-  if (config->copyRelocs)
+  // If --emit-reloc or -r is given, preserve symbols referenced by relocations.
+  if (config->copyRelocs && sym.used)
----------------
referenced by relocations from live sections.


================
Comment at: lld/ELF/Writer.cpp:725
     return;
+  if (config->copyRelocs && config->discard != DiscardPolicy::None)
+    markUsedLocalSymbols<ELFT>();
----------------
We can also add `&& !config->gcSections` here to avoid work.

Now it is clear why we need `--gc-sections` tests in `*-discard-locals`. `Symbol::used` is always true when --no-gc-sections. When --gc-sections is specified, MarkLive sets `used` correctly, another relocation scanning (`markUsedLocalSymbols`) will be unnecessary.


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

https://reviews.llvm.org/D77807