[PATCH] D101080: [lld/mac] Implement support for .weak_def_can_be_hidden

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 22 09:37:28 PDT 2021


thakis created this revision.
thakis added a reviewer: lld-macho.
Herald added a reviewer: int3.
Herald added a project: lld-macho.
thakis requested review of this revision.

I first had a more invasive patch for this (D101069 <https://reviews.llvm.org/D101069>), but while trying
to get that polished for review I realized that lld's current symbol
merging semantics mean that only a very small code change is needed.
So this goes with the smaller patch for now.

This has no effect on projects that build with -fvisibility=hidden
(e.g.  chromium), since these see .private_extern symbols instead.

It does have an effect on projects that build with -fvisibility-inlines-hidden
(e.g. llvm) in -O2 builds, where LLVM's GlobalOpt pass will promote most inline
functions from .weak_definition to .weak_def_can_be_hidden.

Before this patch:

  % ls -l out/gn/bin/clang out/gn/lib/libclang.dylib
  -rwxr-xr-x  1 thakis  staff  113059936 Apr 22 11:51 out/gn/bin/clang
  -rwxr-xr-x  1 thakis  staff   86370064 Apr 22 11:51 out/gn/lib/libclang.dylib
  % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/bin/clang | wc -l
      8291
  % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/lib/libclang.dylib | wc -l
      5698

With this patch:

  % ls -l out/gn/bin/clang out/gn/lib/libclang.dylib
  -rwxr-xr-x  1 thakis  staff  111721096 Apr 22 11:55 out/gn/bin/clang
  -rwxr-xr-x  1 thakis  staff   85291208 Apr 22 11:55 out/gn/lib/libclang.dylib
  thakis at MBP llvm-project % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/bin/clang | wc -l
       725
  thakis at MBP llvm-project % out/gn/bin/llvm-objdump --macho --weak-bind out/gn/lib/libclang.dylib | wc -l
       542

Linking clang becomes a tiny bit faster with this patch:

  x 100    0.67263818    0.77847815    0.69430709    0.69877208   0.017715892
  + 100    0.67209601    0.73323393    0.68600798    0.68917346   0.012824377
  Difference at 95.0% confidence
          -0.00959861 +/- 0.00428661
          -1.37364% +/- 0.613449%
          (Student's t, pooled s = 0.0154648)

This only happens if lld with the patch and lld without the patch are both
linked with an lld with the patch or both linked with an lld without the patch
(...or with ld64). I accidentally linked the lld with the patch with an lld
without the patch and the other way round at first. In that setup, no
difference is found. That makese sense, since having fewer weak imports will
make the linked output a bit faster too. So not only does this make linking
binaries such as clang a bit faster (since fewer exports need to be written to
the export trie by lld), the linked output binary binary is also a bit faster
(since dyld needs to process fewer dynamic imports).

This also happens to fix the one `check-clang` failure when using lld as host
linker, but mostly for silly reasons: See crbug.com/1183336, mostly comment 26.
The real bug here is that c-index-test links all of LLVM both statically and
dynamically, which is an ODR violation. Things just happen to work with this
patch.

So after this patch, check-clang, check-lld, check-llvm all pass with lld as
host linker :)


https://reviews.llvm.org/D101080

Files:
  lld/MachO/InputFiles.cpp
  lld/test/MachO/weak-def-can-be-hidden.s

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101080.339684.patch
Type: text/x-patch
Size: 9396 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210422/d865dd17/attachment.bin>


More information about the llvm-commits mailing list