Skip to content
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

also link to CUDA::cufft_static in case of BUILD_SHARED_LIBS=OFF #3753

Open
wants to merge 1 commit into
base: 4.x
Choose a base branch
from

Conversation

gregorburger
Copy link

@gregorburger gregorburger commented Jun 7, 2024

In case BUILD_SHARED_LIBS is disabled, the cufft library is still linked to the dynamic version, whereas all other cuda libraries are linked to the static versions by appending CUDA_LIB_EXT. This PR makes cufft handles the same way as all other cuda libraries.

Pull Request Readiness Checklist

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@katharinaHochf
Copy link

katharinaHochf commented Jun 7, 2024

In any case someone uses older cmake versions:
CUDA::cufft_static_nocallback starting in CUDA 9.2, requires CMake 3.23+

otherwise static linking does not work even with that fix.

https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#cuda-toolkit-cufft

@gregorburger
Copy link
Author

In any case someone uses older cmake versions: CUDA::cufft_static_nocallback starting in CUDA 9.2, requires CMake 3.23+

otherwise static linking does not work even with that fix.

https://cmake.org/cmake/help/latest/module/FindCUDAToolkit.html#cuda-toolkit-cufft

Should work with the latest changes as well. If the proper versions of cuda or cmake aren't met, it will fall back to dynamic libraries as was the behaviour previously.

Copy link
Contributor

@cudawarped cudawarped left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@gregorburger
Copy link
Author

@cudawarped I'm new to the OpenCV workflow, is there anything missing starting a build on the CI?

@cudawarped
Copy link
Contributor

@cudawarped I'm new to the OpenCV workflow, is there anything missing starting a build on the CI?

No. One of the core maintainers needs to have time to check this is sensible before kicking off the CI. As this is the contrib repo this in not as high a priority.

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

Successfully merging this pull request may close these issues.

None yet

4 participants