KomputeProject / kompute

General purpose GPU compute framework built on Vulkan to support 1000s of cross vendor graphics cards (AMD, Qualcomm, NVIDIA & friends). Blazing fast, mobile-enabled, asynchronous and optimized for advanced GPU data processing usecases. Backed by the Linux Foundation.
http://kompute.cc/
Apache License 2.0
1.95k stars 147 forks source link

Fix the build of the Python bindings. #387

Closed robquill closed 3 weeks ago

robquill commented 1 month ago

When attempting the build the Python bindings like this:

cmake -DKOMPUTE_OPT_BUILD_PYTHON=ON ..
make

I get this error:

[100%] Linking CXX shared module ../lib/kp.cpython-310-x86_64-linux-gnu.so
lto-wrapper: warning: using serial compilation of 2 LTRANS jobs
/usr/bin/ld: ../lib/libfmt.a(format.cc.o): warning: relocation against `stdout@@GLIBC_2.2.5' in read-only section `.text'
/usr/bin/ld: ../lib/libkompute.a(Algorithm.cpp.o): relocation R_X86_64_PC32 against symbol `_ZTINSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEE' can not be used when making a shared object; recompile with -fPIC
/usr/bin/ld: final link failed: bad value
collect2: error: ld returned 1 exit status
make[2]: *** [python/CMakeFiles/kp.dir/build.make:102: lib/kp.cpython-310-x86_64-linux-gnu.so] Error 1
make[1]: *** [CMakeFiles/Makefile2:373: python/CMakeFiles/kp.dir/all] Error 2
make: *** [Makefile:156: all] Error 2
axsaucedo commented 1 month ago

Thank you for the contribution @robquill - this could make sense, but could you expand what this flag would do in context of the broader codebase? Asking as if it's more of a corner case for a particular use-case, it may make more sense adding this on the respective project cmake file as opposed to by default to the repo to all targets

robquill commented 1 month ago

My understanding is that building a shared object requires position-independent code (https://stackoverflow.com/questions/57649452/why-must-shared-libraries-be-position-independent-while-static-libraries-dont), so I could try only setting this option if -DKOMPUTE_OPT_BUILD_PYTHON=ON is set.

I don't think you can only set it e.g. for the Pybind11 module itself as that shared library is still built using the static libraries which would still contain position dependent code.

axsaucedo commented 3 weeks ago

so I could try only setting this option if -DKOMPUTE_OPT_BUILD_PYTHON=ON is set

From my understanding position independent code is glowingly the default in more modern distros, and it may indeed be the case that restricting it to only -DKOMPUTE_OPT_BUILD_PYTHON=ON may be the way to go, as I can't think of a corner case at this stage (unless someone wants to build static vs dynamic version of python although haven't tried if possible)

robquill commented 3 weeks ago

so I could try only setting this option if -DKOMPUTE_OPT_BUILD_PYTHON=ON is set

From my understanding position independent code is glowingly the default in more modern distros, and it may indeed be the case that restricting it to only -DKOMPUTE_OPT_BUILD_PYTHON=ON may be the way to go, as I can't think of a corner case at this stage (unless someone wants to build static vs dynamic version of python although haven't tried if possible)

OK then, that is already done, so I think this should be good to go.