[PATCH] D142484: [LinkerWrapper] Only import static libraries with needed symbols

Tue Jan 24 10:29:02 PST 2023

jhuber6 added a comment.

In D142484#4077811 <https://reviews.llvm.org/D142484#4077811>, @tra wrote:

> @MaskRay - we seem to be reinventing the linker here and could use your expertise.

Yeah, this reinvents a lot of logic. But I don't think there's an easy way to get around this without duplicating some ugly logic. I would absolutely love to shove the vast majority of this LTO / static library handling directly into the linker. E.g. we package the extracted libraries into a new one and let `lld` handle it. The problem is pretty firmly in Nvidia's hands because they don't have a real linker. They use `nvlink` which does some magic to `.cubin` files and doesn't understand our LTO or static library linking. So in order to make this common I needed to reproduce a lot of that logic here.

================
Comment at: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp:1217-1218
+///   1) It defines an undefined symbol in a regular object filie.
+///   2) It defines a global symbol without hidden visibility that has not
+///      yet been defined.
+Expected<bool> getSymbols(StringRef Image, StringSaver &Saver,
----------------
tra wrote:
> How do we handle conflicting symbols defined more than once? 
> 
I just assume that will be caught by the actual linker, it's not relevant for knowing which symbols to pull out.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142484