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

Fix: Consistent Usage of target_link_libraries for CUDA Targets #646

Merged
merged 2 commits into from
Jan 26, 2024

Conversation

basnijholt
Copy link
Contributor

@basnijholt basnijholt commented Dec 4, 2023

This pull request addresses a build configuration issue that arises (introduced in #643) when mixing plain and keyword signatures of target_link_libraries in CMake for the qsim_cuda, qsim_custatevec, qsim_decide targets. The existing setup led to a CMake error, as documented:

CMake Error at pybind_interface/cuda/CMakeLists.txt:48 (target_link_libraries):
  The plain signature for target_link_libraries has already been used with
  the target "qsim_cuda".  All uses of target_link_libraries with a target
  must be either all-keyword or all-plain.

Changes Made:

  • Modified the target_link_libraries command for the qsim_cuda, qsim_custatevec, qsim_decide targets to use the plain form. This change aligns with the internal usage of target_link_libraries within the cuda_add_library macro, ensuring consistency across the build configuration.

Rationale:

CMake requires that all invocations of target_link_libraries for a specific target maintain a consistent usage pattern - either all using the plain form or all using the keyword form (such as PUBLIC or PRIVATE). The cuda_add_library macro used in this project employs the plain form of target_link_libraries, which necessitates a similar approach for additional invocations of target_link_libraries for the qsim_cuda, qsim_custatevec, qsim_decide targets.

By aligning the usage pattern, this pull request resolves the build configuration error and maintains consistency in the CMake setup, ensuring a smoother build process.

Otherwise this could lead to the following problem when building with CUDA:
```
  CMake Error at pybind_interface/cuda/CMakeLists.txt:30 (target_link_libraries):
    The plain signature for target_link_libraries has already been used with
    the target "qsim_cuda".  All uses of target_link_libraries with a target
    must be either all-keyword or all-plain.

    The uses of the plain signature are here:

     * /usr/share/cmake-3.16/Modules/FindCUDA.cmake:1836 (target_link_libraries)
```
@basnijholt basnijholt changed the title Remove PUBLIC from target_link_libraries with OpenMP::OpenMP_CXX Fix: Consistent Usage of target_link_libraries for CUDA Targets Dec 4, 2023
@basnijholt basnijholt marked this pull request as ready for review December 4, 2023 20:26
@NoureldinYosri NoureldinYosri added the kokoro:run Trigger Kokoro builds for this PR. label Jan 26, 2024
@qsim-qsimh-bot qsim-qsimh-bot removed the kokoro:run Trigger Kokoro builds for this PR. label Jan 26, 2024
@basnijholt
Copy link
Contributor Author

basnijholt commented Jan 26, 2024

@NoureldinYosri note that the CUDA build is currently broken and these changes are required to make it work.

@NoureldinYosri NoureldinYosri merged commit 83839cc into quantumlib:master Jan 26, 2024
16 checks passed
basnijholt added a commit to basnijholt/qsimcirq-feedstock that referenced this pull request Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants