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

Mon Jun 10 16:02:34 PDT 2019

steven_wu added a comment.

> 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.

It would be good if the API is flexible to return either. Not against having a different API to return linker options but I think that is less clean.

> 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?

I agree it should be in the linker implementation.

> 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.

This is fine. Do you think you might want to query symbols from InputFile instead of LTOModule in the future?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62935