-
Notifications
You must be signed in to change notification settings - Fork 3.9k
[cmake] consolidate set_target_properties() calls #6594
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
Conversation
lightgbm_objs _lightgbm | ||
PROPERTIES | ||
CUDA_ARCHITECTURES ${CUDA_ARCHS} | ||
CUDA_SEPARABLE_COMPILATION ON |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CUDA_SEPARABLE_COMPILATION
was not set for _lightgbm
target in previous config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah! Thank you for catching that, I've updated it in recent commits.
This also led me to question what this property actually does. Here's a simple explanation of it: https://stackoverflow.com/a/50557765/3986677
endif() | ||
set_target_properties(_lightgbm PROPERTIES CUDA_RESOLVE_DEVICE_SYMBOLS ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was lost in new config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah! Too many similarly-named properties 😂
Thank you for catching this, I've fixed that in recent commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for solving this CMake puzzle! 😄
Sure! I've been learning a lot about CMake lately, hoping to continue simplifying and improving LightGBM's CMake stuff. I'm especially interested in making LightGBM more friendly to build and link to for repackagers like @barracuda156 (see for example #6489). |
Another step towards simplifying this project's CMake configuration (are you sensing a theme in my recent PRs? 😂 ).
CMake's
set_target_properties()
accepts multiple targets and multiple properties (CMake docs).This proposes taking advantage of that to consolidate a few calls.