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 cmake cuda_arch option #2553

Merged
merged 8 commits into from Mar 29, 2017

Conversation

Projects
None yet
3 participants
@atrantan
Copy link
Contributor

commented Mar 21, 2017

This patch tries to give a means for the user to explicitely define the different gpu architectures for which device code has to be generated. Example : cmake $HPX_ROOT -DHPX_WITH_CUDA=true -DHPX_WITH_CUDA_ARCH=sm_20,sm_21

@hkaiser

This comment has been minimized.

Copy link
Member

commented Mar 21, 2017

@atrantan I know this is superficial, but could you please rename the new option to HPX_WITH_CUDA_ARCH?

SET_SOURCE_FILES_PROPERTIES(${source} PROPERTIES LANGUAGE CXX)
SET_SOURCE_FILES_PROPERTIES(${source} PROPERTIES
LANGUAGE CXX
COMPILE_FLAGS ${HPX_CUDA_CLANG_FLAGS})
endif()

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 21, 2017

Member

Shouldn't this be added to hpx_setup_target instead. The user might be interested in using CUDA code from within a shared or static library...

This comment has been minimized.

Copy link
@atrantan

atrantan Mar 21, 2017

Author Contributor

Here is a trick to manage files that has .cu extension because with clang we are not using cuda_add_executable() but the native add_executable(). SET_SOURCE_FILES_PROPERTIES() was already used in master

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 22, 2017

Member

Sure, but the .cu file could be part of a shared or a static library. So adding it to setup_target would enable the required flags for those cases as well, not only for compiling a .cu file into an executable.

This comment has been minimized.

Copy link
@atrantan

atrantan Mar 22, 2017

Author Contributor

@hkaiser When implementing your suggestion, got some warnings when source files are .cpp files: clang-5.0: warning: argument unused during compilation: '--cuda-gpu-arch=sm_20' [-Wunused-command-line-argument]

This comment has been minimized.

Copy link
@sithhell

sithhell Mar 22, 2017

Member

You have to go through the source list and only append the flags to the files with a .cu extension.

This comment has been minimized.

Copy link
@atrantan

atrantan Mar 22, 2017

Author Contributor

@sithhell Was my suggestion

This comment has been minimized.

Copy link
@hkaiser

hkaiser Mar 22, 2017

Member

@atrantan Sure, but I think you should do that in setup_target and not in add_executable.

This comment has been minimized.

Copy link
@atrantan

atrantan Mar 22, 2017

Author Contributor

@hkaiser Done in f4271a9

@hkaiser

This comment has been minimized.

Copy link
Member

commented Mar 21, 2017

@atrantan The same goes for HPX_WITH_CUDA_CLANG_FLAGS, I guess...

@hkaiser

This comment has been minimized.

Copy link
Member

commented Mar 22, 2017

@atrantan While your proposed solution is now doing the right thing, it still is not quite what we should do. You added the SOURCES as a mandatory argument to setup_target. This would break the intent behind setup_target which was for it to be usable by external projects in conjunction with the native cmake macros like add_executable et.al. I think that inside setup_target you can retrieve the list of sources from the target using get_target_properties(target PROPERTIES SOURCES variable) instead.

@hkaiser
Copy link
Member

left a comment

LGTM, thanks!

@atrantan atrantan force-pushed the atrantan:add_cmake_cuda_arch_option branch from 1fdee0c to 4c29d01 Mar 28, 2017

@hkaiser hkaiser merged commit 93f88a0 into STEllAR-GROUP:master Mar 29, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.