[PATCH] D62935: [ThinLTO][LTO][Legacy] Fix dependent libraries support by adding querying of the IRSymtab

[ben via Phabricator via llvm-commits]

bd1976llvm added a comment.

Thanks for the response.

In D62935#1534216 <https://reviews.llvm.org/D62935#1534216>, @steven_wu wrote:

> This is ELF specific and miss the initial RFC so there is quite some reading to catch up. Should we merge the MachO implementation together with the ELF one? Once you commit this, you might want to file a bug report on me to catch up on the macho side.


We should look at either merging the implementations or providing an api function that provides access to the "llvm.linker.options" named metadata via the IRSymtab. It may not be possible to merge the implementations because in COFF and MACHO the representation of dependent libraries is a string of command line options; whereas, the ELF representation is an array of library strings. This is because for COFF and MACHO there is basically only one linker for each file format so the command line format is fixed. For ELF there are a greater variety of linkers, so I just pass the library strings through to the linker, without processing them into command line arguments, and it is deferred to the linker to interpret the library strings.

> Can you also add an integrated testcase (with llvm-lto) to show how this should be integrated with rest of the pipeline? There can be some interesting corner cases, like pulling in a static library with another dependency library (which might or might not already satisfied).

I might be misunderstanding here, but I don't think I can provide any further testing. These api functions are intended to be used by a client linker so that it can do symbol resolution (including processing the dependent libraries) on bitcode files. This is done before invoking the Full/ThinLTO codegen to generate native object files from the input bitcode files. So all we need to do is test that the correct information is supplied to a client linker. How to resolve scenarios such as pulling in a static library which specifies a dependent library is up to the client linker, no?

> From the first glance, the patch looks fine to me.

Thanks. I was not sure if the new functions in LTOModule.h/.cpp were in the right file - but I don't see a better place to put them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62935