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

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added inline comments.


================
Comment at: lld/ELF/Writer.cpp:725
     return;
+  if (config->copyRelocs && config->discard != DiscardPolicy::None)
+    markUsedLocalSymbols<ELFT>();
----------------
MaskRay wrote:
> 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.
I've added the check and the comment to `markUsedLocalSymbols<ELFT>()`.


================
Comment at: lld/test/ELF/emit-relocs-discard-locals.s:27
+
+## --discard-locals removes unused local symbols which start with ".L"
+# DISCARD-LOCALS:    0: {{0+}} 0 NOTYPE  LOCAL  DEFAULT UND
----------------
MaskRay wrote:
> I think you can still combine the two chunks of CHECK lines. 
> 
> `DICARD-LOCALS-NEXT:` for the common part.
> `DISCARD-LOCALS-NOGC` for the additional lines by --no-gc-sections.
> 
> It makes the diff stand out.
OK, combining gc/nogc variants together looks acceptable. Firstly, I tried to combine all four variants and the result was unreadable, that is why I split them.


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

https://reviews.llvm.org/D77807