[PATCH] D132799: Make sure libLLVM users link with libatomic if needed

[Aaron Puchert via Phabricator via llvm-commits]

aaronpuchert added a comment.

In D132799#3779971 <https://reviews.llvm.org/D132799#3779971>, @beanz wrote:

> The Z3 Solver is an external dependency that gets put on any user of libLLVM if they are building with Z3, so that's correct too.

Hmm, could you elaborate why? All I can find is

  llvm/lib/Support/Z3Solver.cpp:#include <z3.h>

so that doesn't look like a public dependency to me. That's different from libatomic, which is used via `llvm/ADT/Statistic.h`, so users of libLLVM have to link with that directly if it's required.

Same goes for some other "system libs":

  llvm/lib/Support/CRC.cpp:#include <zlib.h>
  llvm/lib/Support/Compression.cpp:#include <zlib.h>
  
  llvm/lib/Support/Compression.cpp:#include <zstd.h>



In D132799#3784308 <https://reviews.llvm.org/D132799#3784308>, @phosek wrote:

> I think a better solution would be to introduce imported CMake targets for system libraries like libatomic (and use the existing supports where it already exists like the threads library <https://cmake.org/cmake/help/latest/module/FindThreads.html>) and use CMake dependencies as needed.

The issue isn't finding the library, but propagating it to libLLVM as public dependency, i.e. **users** of libLLVM will have to link with with libatomic. When it comes to linking libLLVM itself we already include it, but we don't use public dependencies on shared libraries at all:

  if(ARG_STATIC)
    set(libtype PUBLIC)
  else()
    # We can use PRIVATE since SO knows its dependent libs.
    set(libtype PRIVATE)
  endif()

which seems like a faulty assumption to me. Directly using functionality from dependent libs still requires specifying them on the linker command line.

> The `SYSTEM_LIBS` concept in LLVM build is only used by `llvm-config` and I expect we'll remove it entirely once we finally manage to deprecate `llvm-config`.

Perhaps `SYSTEM_LIBS` is also too big for this. It seems to contain a number of private dependencies. I'm thinking of reverting this change and going back to my original suggestion, but want to hear from @beanz first.

Ideally we would properly track private vs. public link dependencies, and then we could collect them from the component libraries into libLLVM. But that would be quite some work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132799