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

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin added a comment.

In D77807#1989361 <https://reviews.llvm.org/D77807#1989361>, @MaskRay wrote:

> > Implement --discard-* for cases when -r or --emit-relocs are used.
>
> The behavior difference should only be about `--discard-all` + `{-r,--emit-relocs}`.
>
> For `--discard-locals` + `{-r,--emit-relocs}` (`.Lunused` is stripped in GNU ld), I am not sure we want to adapt GNU ld's rule.


And what should we do instead? Note that without `-r` or `--emit-relocs` our behavior matches GNU linkers'.

> For `--discard-all` + `{-r,--emit-relocs}` (the main purpose of this patch, I think. `.Lunused` and `unused` are stripped in GNU ld), I am still uncomfortable with the following change:
> 
>   - used(!config->gcSections),
>   + used(!config->gcSections && binding != llvm::ELF::STB_LOCAL),

Do you suggest to extend the expression and include `config->copyRelocs` into it, or revert this change and stick with my initial version, which was more localized?

> and the relocation scanning behavior guarded by the subtle condition:
> 
>   if (config->copyRelocs && config->discard != DiscardPolicy::None)
>     markUsedLocalSymbols<ELFT>();
>    

The condition is here just to avoid unneeded work. Any suggestions on how to improve that?


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

https://reviews.llvm.org/D77807