Skip to content

build: Allow building ROOT using the Clang shared library #17698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jakeforster opened this issue Feb 12, 2025 · 10 comments
Open

build: Allow building ROOT using the Clang shared library #17698

jakeforster opened this issue Feb 12, 2025 · 10 comments

Comments

@jakeforster
Copy link

Explain what you would like to see improved and how.

Hello gentle maintainers

My understanding is PR #15563 added support for building Cling as a standalone package using shared libraries from LLVM and Clang. This was facilitated by adopting the new style for embedding LLVM in a project (see https://llvm.org/docs/CMake.html#embedding-llvm-in-your-project).

Similarly, does ROOT's build system allow ROOT to be built using shared libraries from LLVM and Clang? If not, would you be interested in modifying it to support this?

As discussed in that previous PR, there is an advantage to allowing these builds to be done separately, and I assume this modification could be done such that ROOT can still be built with the bundled LLVM and Clang when the builtin options are passed.

For reference, my attempt to build ROOT with external LLVM, Clang and Cling fails at the 'rootcling_stage1' target, which I'm assuming is due to the expectation of building using static libraries (though I'd be happy to be wrong):

ld: /gnu/store/vy8c11s0svpy3hy6g31a704f23cllhj1-cling-1.1/lib/libclingInterpreter.a(LookupHelper.cpp.o):(.text._ZN5clingL18ParseWithShortcutsEPN5clang11DeclContextERNS0_10ASTContextEN4llvm9StringRefEPNS_11InterpreterERNS0_13UnqualifiedIdENS_12LookupHelper11DiagSettingERNS_15ParserStateRAIIE+0x4e5): undefined reference to `clang::Sema::RequireCompleteDeclContext(clang::CXXScopeSpec&, clang::DeclContext*)'
ld: /gnu/store/vy8c11s0svpy3hy6g31a704f23cllhj1-cling-1.1/lib/libclingInterpreter.a(LookupHelper.cpp.o): in function `cling::LookupHelper::findScope(llvm::StringRef, cling::LookupHelper::DiagSetting, clang::Type const**, bool) const':
(.text._ZNK5cling12LookupHelper9findScopeEN4llvm9StringRefENS0_11DiagSettingEPPKN5clang4TypeEb+0x12b9): undefined reference to `clang::Sema::RequireCompleteDeclContext(clang::CXXScopeSpec&, clang::DeclContext*)'
collect2: error: ld returned 1 exit status
make[2]: *** [core/rootcling_stage1/CMakeFiles/rootcling_stage1.dir/build.make:167: core/rootcling_stage1/src/rootcling_stage1] Error 1

That's from building ROOT version 6.32.08 without builtin Cling, LLVM and Clang; LLVM is from tag "ROOT-llvm16-20240621-01" of root-project's fork, Clang is built from that same LLVM, and Cling is version 1.1 built with LLVM from tag "cling-llvm16-20240621-02".

Thanks in advance!
Jake

ROOT version

6.32.08. Also tried with 6.34.02 using Cling 1.2 and LLVM/Clang 18.

Installation method

From source

Operating system

Linux (Guix)

Additional context

No response

@hahnjo
Copy link
Member

hahnjo commented Feb 12, 2025

I think this is related to #17461 where the solution to both issues may be to use find_package(Clang).

@hahnjo
Copy link
Member

hahnjo commented Feb 17, 2025

Hi @jakeforster, #17737 fixes #17461. This should get us closer to potentially allow using the Clang shared library, but there are two blockers:

  1. I was reminded why we currently don't allow LLVM_LINK_LLVM_DYLIB: Both LLVM and rootcling defines the quite generic -W option and when linking with libLLVM.so you get : CommandLine Error: Option 'W' registered more than once!.
  2. In order to use libclang-cpp.so (this is the library you mean, right?), we would need to replace all individual clang* dependencies by clang-cpp. From looking at AddClang.cmake, this seems to happen in clang_target_link_libraries but I'm not sure we have that available in all places we need it...

So some progress, but not quite. If you want to give it a try with fresh eyes from your side, please let us know how it goes!

@jakeforster
Copy link
Author

Hi @hahnjo

Thank you very much for your work toward improving the build system! Sorry I haven't been able to help so far.

I saw in #17737 that you think it might be ready for more testing, so I tried again.

For ROOT's inputs, I used:

  • Cling 1.2 (built using LLVM from tag "cling-llvm18-20240821-01" and Clang built with the same LLVM)
  • LLVM from tag "ROOT-llvm18-20250313-01" - which I refer to as "llvm-root"
  • Clang built using LLVM from tag "ROOT-llvm18-20250313-01" - which I refer to as "clang-root"

I started with ROOT commit 3c0c962 and applied a couple of patches (see master...jakeforster:root:allow-building-against-shared-llvm-library-ctd) to get as far as the get_target_property errors shown below.
Full log: root-build-log.txt.

Thanks! Please let me know if I can try anything out.

CMake Warning at interpreter/CMakeLists.txt:10 (message):
  Due to ROOT-specific patches you need a special version of clang.  You
  cannot use vanilla clang.


-- Could NOT find FFI (missing: FFI_LIBRARIES HAVE_FFI_CALL)
-- Found LLVM 18.1.8 in /gnu/store/sm245x42hj6svyg40f0gg3zi3r7h05dc-llvm-root-18-20250313-01/lib/cmake/llvm
-- Linker detection: GNU ld
-- Building with -fPIC
-- Using LLVM external library - 18.1.8
-- Could NOT find FFI (missing: FFI_LIBRARIES HAVE_FFI_CALL)
-- Found Clang  in /gnu/store/kgryk9iq7snh56i8w2z6062kxjvp397b-clang-root-18-20250313-01/lib/cmake/clang
-- Could NOT find FFI (missing: FFI_LIBRARIES HAVE_FFI_CALL)
-- Building with -fPIC
-- CLING_INCLUDE_DIRS: /gnu/store/7zbn4vfi56w1vpi2hpc5rm90nkgpiibg-cling-1.2/include
-- CLANG_INCLUDE_DIRS: /gnu/store/kgryk9iq7snh56i8w2z6062kxjvp397b-clang-root-18-20250313-01/include
-- LLVM_INCLUDE_DIRS: /gnu/store/sm245x42hj6svyg40f0gg3zi3r7h05dc-llvm-root-18-20250313-01/include
-- LLVM_DEFINITIONS_LIST:
CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "dl".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "dl".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMObject>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMOrcShared>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMOrcTargetProcess>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMSupport>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMTargetParser>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "rt".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMAnalysis>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMBitReader>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMBitWriter>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target
  "$<LINK_ONLY:LLVMPasses>".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "rt".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "dl".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "m".


CMake Error at interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt:93 (get_target_property):
  get_target_property() called with non-existent target "rt".

@hahnjo
Copy link
Member

hahnjo commented Mar 27, 2025

@aaronj0 @vgvassilev looks like CppInterOp made things worse...

@guitargeek
Copy link
Contributor

This PR will improve the situation with CppInterOp:
#18386

The problem was that find_package(Clang) is setting back LLVM_LINK_LLVM_DYLIB=TRUE if LLVM was built with that setting. So we need to set it to false again afterwards.

@guitargeek
Copy link
Contributor

Hi @hahnjo! FYI, with your revert, I think we went back to the situation where "CppInterOp made things worse" as you said.

@hahnjo
Copy link
Member

hahnjo commented Apr 30, 2025

Yes, as expected. I had a quick look and the code that throws the errors is in interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt which has a long section for LLVM_LINK_LLVM_DYLIB described as Get rid of libLLVM-X.so which is appended to the list of static libraries. @aaronj0 can you maybe check that code? It's not clear to me why it is needed because the same file also uses DISABLE_LLVM_LINK_LLVM_DYLIB...

@vgvassilev
Copy link
Member

Yes, as expected. I had a quick look and the code that throws the errors is in interpreter/CppInterOp/lib/Interpreter/CMakeLists.txt which has a long section for LLVM_LINK_LLVM_DYLIB described as Get rid of libLLVM-X.so which is appended to the list of static libraries. @aaronj0 can you maybe check that code? It's not clear to me why it is needed because the same file also uses DISABLE_LLVM_LINK_LLVM_DYLIB...

Because the cmake of shipped llvm are broken. It does not matter if we specify this option, if llvm is shipped in a package manager we have a problem as the cmake config or the cmake targets forces an explicit linking of libLLVM.so.

@hahnjo
Copy link
Member

hahnjo commented Apr 30, 2025

DISABLE_LLVM_LINK_LLVM_DYLIB allows overriding LLVM_LINK_LLVM_DYLIB (see also its documentation in llvm_add_library), and so far I have not been presented an example where this didn't work...

@vgvassilev
Copy link
Member

DISABLE_LLVM_LINK_LLVM_DYLIB allows overriding LLVM_LINK_LLVM_DYLIB (see also its documentation in llvm_add_library), and so far I have not been presented an example where this didn't work...

Does not work for shipped llvm on conda. Tracing this down is very hard but I did that once (and forgot most of it) but basically somewhere in the shipped cmake files with the llvm release there is logic which disregards this option and hardcodes libLLVM.so and CppInterOp.so starts loading libLLVM.so and we get the usual assert.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants