kito-cheng accepted this revision.
kito-cheng added a comment.
This revision is now accepted and ready to land.
ld's help message just confused me, that say `-X` is default, but actually
default action is `Discard local temporary symbols in SEC_MERGE sections.`
which is no option can enable that but default.
$ ld.bfd --help
...
-x, --discard-all Discard all local symbols
-X, --discard-locals Discard temporary local symbols (default)
--discard-none Don't discard any local symbols
...
Anyway, back to the RISC-V part, that set `-X (--discard-locals)` IF discard
policy is default, which match the behavior of `emultempl/riscvelf.em`, however
I've little concern about that might change the behavior for `ld.bfd` when we
try to link with `clang -target riscv64 -Wl,-r -Wl,-s`: ld will treat `ld -r
-s` as `ld -r -S -x`, but this change changed this behavior because we always
append a `-X` here, so it the behavior is become treat `ld -r -s` as `ld -r
-S`, but I must say I really not sure how important of this behavior - the
behavior here is over 20 years[2, 3] and I didn't found any document to
describe why.
But generally I believe discard fewer symbol isn't harmful - just made the
object file fatter little bit, so I give a LGTM here.
[1]
https://github.com/bminor/binutils-gdb/blob/master/ld/emultempl/riscvelf.em#L34
[2]
https://github.com/bminor/binutils-gdb/blob/252b5132c753830d5fd56823373aed85f2a0db63/ld/ldmain.c#L272
[3]
https://github.com/bminor/binutils-gdb/commit/f5fa8ca231d47662321addbfbde105e2bed0be07#diff-3b7cfc539d765310991a11f7f328f095fc6f9e17fea1a8510c949c210804deb5R281
/* Treat ld -r -s as ld -r -S -x (i.e., strip all local symbols). I
don't see how else this can be handled, since in this case we
must preserve all externally visible symbols. */
if (bfd_link_relocatable (&link_info) && link_info.strip == strip_all)
{
link_info.strip = strip_debugger;
if (link_info.discard == discard_sec_merge)
link_info.discard = discard_all;
}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D127826/new/
https://reviews.llvm.org/D127826
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
