Linking with all LLVM libraries #53
Conversation
|
There is a corresponding PR for ocl-open-80 branch - #54 |
|
#54 works for me when I link against statically linked LLVM. I will try to check now dynamic linkage against LLVM. This will take a while, I need to rebuild it... |
| set(ADDITIONAL_LIBS ) | ||
|
|
||
| if(USE_PREBUILT_LLVM AND NOT LLVMSPIRV_INCLUDED_IN_LLVM) |
There was a problem hiding this comment.
This will cause linker error for LLVMSPIRVLib for systems that have LLVMSPIRV built into libLLVM.so
|
I'm in the process of testing by compiling intel-compute-runtime for dynamic llvm libs case. |
|
Building common_clang against dynamic LLVM (built with -DLLVM_BUILD_LLVM_DYLIB=ON) works fine as well for me. So, regarding this PR I have nothing against, but you may wish to address comment from @tripzero to allow builds against LLVM which include SPIRV translator. However, I am not quite sure that this would be a common practice among distributions since/till SPIRV translator won't show up as a standard tool included into default LLVM distributions, for example, appearing here: https://github.com/llvm/llvm-project. For that matter, Debian approach to package SPIRV translator separately https://salsa.debian.org/opencl-team/spirv-llvm-translator seems to be more native for me. A bit of bad news however. So, common clang build is ok. IGC is also ok. Problem happens with NEO opencl driver. Configuring LLVM in this way: I see this: It is coming from here: I should note that I surprisingly don't see this issue when I build against static LLVM, i.e. with And the stack leading to that looks to be: So, here https://github.com/llvm/llvm-project/blob/release/8.x/llvm/lib/Target/AMDGPU/AMDGPUTargetMachine.cpp#L264 there is some globals initialization in LLVM which error out: Can that be that we have initialized something already within other DLL loading? |
|
I also see a core dump when using ocloc, but mine appears to be slightly different: |
|
@tripzero : I saw such code dumps when ocloc or one of igc libraries fails to dlopen() another library. Mind that ocloc loads libigdrcl.so, libigc.so, libcommon_clang.so and other libs from LLVM. They need to be in a libraries search path. Moreover, you won't be able to adjust it via LD_LIBRARY_PATH since there are these constructions for ocloc: "LD_LIBRARY_PATH=/path/to/comput_runtime/location ocloc..." |
|
@dvrogozh right, I was missing the dependency on opencl-clang within compute-runtime. I don't get any assertions/crashes now. |
|
In case of Release build of LLVM instead of assert I see this error: |
|
@dvrogozh could you run cloc with "LD_DEBUG=files" in front of the command line. |
|
@AlexeySotkin : that was the first time I attempted building LLVM into dynamic library and I was looking here for the options: http://llvm.org/docs/GettingStarted.html. This page lists LLVM_BUILD_LLVM_DYLIB. However, I saw at least in ClearLinux LLVM .spec file that they use LLVM_LINK_LLVM_DYLIB instead as you suggested. I am not sure what's the difference. So, I was curious. I trusted official LLVM document. I will try to rebuild with LLVM_LINK_LLVM_DYLIB today and see what will happen. |
|
@AlexeySotkin : here is ocloc rerun with LD_DEBUG=files: |
|
By the way, libLLVM-8.so is encountered at least twice in dependencies resolution: And as you can see in LD_DEBUG=files log, libLLVM-8 was loaded by libigdfcl,so.1 and was not loaded for libcommon_clang.so.8 since it was already there. I wonder, this 2nd faulty registering of parser comes for libLLVM.so or from something else? |
|
@dvrogozh, looking at Building LLVM with CMake looks like for common_clang build purposes both of these options should be the same:
|
|
There is something which I don't quite understand and which bothers me. So, I built LLVM with LLVM_BUILD_LLVM_DYLIB=ON. Still, I got both libLLVM-8.so and a bunch of lib*.a files. Moreover, when I run this, I get that I need to link all the static libs, it does not mention .so: And here is link command line for common_clang (sorry, it is a long one): If you will search thru it, you will see that it has /home/dvrogozh/git/github/tmp/install.llvm8/lib/libLLVM-8.so and a bunch of .a files as well. Can that be that we somehow link the same twice? |
|
It seems I root caused it. So, I tried 2 experiments hacking the above common_clang linker command line:
Thus, it seems that we can not link llvm component libLLVM*.a and libLLVM-8.so at the same time. It seems that this behavior was caused by add_llvm_library() since I don't see that we have any command to link libLLVM-8.so or libLLVM*.a explicitly. I thought that this macro is capable to correctly handle situation by itself, but it seems it is not... I wonder, is that correct thing to do to use add_llvm_library()? Maybe the purpose of it is to implement new llvm component which don't depend on others and it is not applicable in case when we try to create a library which uses other components? |
|
As of now I don't see other way to distinguish between 2 kinds of LLVM builds other than check it on project's cmake level. In a way that's project decision how it wants to link against LLVM: statically or dynamically. I believe that nice default would be link against libLLVM-8.so if it is available and link against .a if it is not. We might consider to introduce a build option to force one way or another. In case of the default described above, I did this: So, @AlexeySotkin : can you, please, review this proposal? |
Also adding " -Wl,--no-undefined" to the link flags.
I'm not sure, but this is probably caused by a some And such linkage is actually performed here: https://github.com/llvm-mirror/llvm/blob/master/cmake/modules/AddLLVM.cmake#L550 Did you perform clean-build when you switched between
AFAIK, each of LLVM library (i.e. Support, IR, etc.) is built using exactly this functionality: creating a new component with dependencies on some of the already existing |
There was a problem hiding this comment.
@dvrogozh, could you please re-check this pull request using clean build of LLVM?
|
@AlexeySachkov : all experiments which I did were done with clean build of LLVM. I think you are mistaken regarding add_llvm_library().
You don't use this function, you use add_llvm_library() instead of llvm_add_library() which you have referenced. However, I have tried to use the latter one - this don't help.
This phrase means that library we produce (libcommon_clang) will be marked to be linked inside libLLVM during LLVM build. However, this is not our case: we don't need to build libLLVM into libLLVM.so, instead we are trying to produce libcommon_clang.so. And DISABLE_LLVM_LINK_LLVM_DYLIB can be used to disable the behavior, i.e. make sure that libLLVM.so will not include inside our library. |
|
I finally tried to build with -DLLVM_LINK_LLVM_DYLIB=ON and it really works! So, indeed this flag is being propagated into LLVM installed environment and is visible for the end-users via LLVMConfig.cmake. Indeed, this flag permits to tell that libraries should be built against libLLVM-8.so and not against .a static libs. So, we have 3 build variants and the following status with current state of PR:
Distributions seem to prefer variant 3). For example, here is Debian packaging instructions: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/blob/8/debian/rules#L358. For distributions which prefer 1) or 3) current PR should be fine. @tripzero, @tjaalton : FYI
Anyhow, we can distinguish 2) with the following check which is similar to what I suggested above: I am not sure about priority of fixing that. It might be ok to merge the change as is and clarify with LLVM developers without a rush. |
|
I posted a question here: http://lists.llvm.org/pipermail/llvm-dev/2019-March/131252.html |
Also adding " -Wl,--no-undefined" to the link flags.