[PATCH] D134637: clang-tblgen build: avoid duplicate inclusion of libLLVMSupport

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a reviewer: beanz.
mstorsjo added a subscriber: beanz.
mstorsjo added a comment.

Adding @beanz who might have some more cmake knowledge on whether this is the best/least bad way of doing things.



================
Comment at: clang/lib/Support/CMakeLists.txt:26
+    DISABLE_LLVM_LINK_LLVM_DYLIB
+    ${clangSupport_sources})
+endif()
----------------
DavidSpickett wrote:
> nhaehnle wrote:
> > DavidSpickett wrote:
> > > Can you detail which targets link to/include what and how the problem happens? I'm trying to understand why we can't just use `DISABLE_LLVM_LINK_LLVM_DYLIB` on its own here.
> > clangSupport is included by clang-tblgen but also by libclangcpp. The underlying idea is that of all the users of clangSupport, clang-tblgen is special because it uses the DISABLE_LLVM_LINK_LLVM_DYLIB override.
> > 
> > clangSupport links against Support, which becomes a link against libLLVM-*.so with LLVM_LINK_LLVM_DYLIB=ON. So, in an LLVM_LINK_LLVM_DYLIB=ON build, we get with this change:
> > 
> > - clangSupport links against Support, which becomes dynamically linking against libLLVM-*.so (this is unchanged)
> > - clangSupport_tablegen links against Support statically
> > - clang-tblgen links against clangSupport_tablegen (and also directly against Support) statically
> > - other users of clangSupport link against clangSupport somehow, and then transitively dynamically against libLLVM-*.so
> > 
> > Does that answer your questions?
> > 
> > Specifically, if we were to just add DISABLE_LLVM_LINK_LLVM_DYLIB to clangSupport, then we risk a situation where some other user of clangSupport links against Support twice; once via the copy of Support that is statically linked to from clangSupport; and once via libLLVM-*.so that gets pulled in via other dependencies. To be honest, I don't know for certain whether that is a problem that would happen, but it seemed likely enough to me that I wouldn't want to risk it.
> > Does that answer your questions?
> 
> Yes but I don't think I have the expertise to say this is a good way to implement this change.
FWIW I did try to implement something similar for some of the MLIR tablegen tools too, and I ended up doing something very similar (splitting support libraries into a regular and static-only variant, but for a deeper hierarcy of libraries) - but I haven't taken time to try to upstream that yet. If we get this as generic pattern for how to do it, I could try to upstream those patches later too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134637