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

Add CUDA libraries when linking if CUDA is found by cmake #545

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

mewall
Copy link
Collaborator

@mewall mewall commented Oct 6, 2021

No description provided.

@jmohdyusof
Copy link
Collaborator

Still curious if manually appending to the list or using a function like include_directories() is preferred.

@nicolasbock
Copy link
Collaborator

The function include_directories() adds to the list of includes, i.e. is used for header files. The LINK_LIBRARIES variable tracks part of the linker flags. Both would be needed to add a library.

But as @mewall points out, the FindCUDA module is deprecated since CMake 3.10. Since we are requiring at least that version it's safe to drop the code around the FindCUDA module and replace it with listing CUDA as a compiler language at the top of the CMakeLists.txt file.

Copy link
Collaborator

@nicolasbock nicolasbock left a comment

Choose a reason for hiding this comment

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

It is cleaner to just remove the FindCUDA module and all of the logic that goes with it and replace it with listing CUDA as a language at the top.

@mewall
Copy link
Collaborator Author

mewall commented Oct 6, 2021 via email

@jmohdyusof
Copy link
Collaborator

@nicolasbock What I was trying to say was that maybe there should be a link_directories() function that adds to the link_libraries. It seems incongruous to use two different methods to do very similar things (since the include_directories should also append to some list of include paths)

@nicolasbock
Copy link
Collaborator

nicolasbock commented Oct 12, 2021

Actually there is target_link_libraries which looks like it is exactly what you were thinking of @jmohdyusof . I can't remember why we don't use that call to be honest. It looks a lot cleaner than adding to LINK_LIBRARIES directly 😄

@nicolasbock
Copy link
Collaborator

rebased

@nicolasbock nicolasbock merged commit a7ef960 into master Oct 12, 2021
@nicolasbock nicolasbock deleted the cuda_fix_revised branch October 12, 2021 19:53
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.

4 participants