Add cmake option for CAMP_USE_PLATFORM_DEFAULT_STREAM#205
Add cmake option for CAMP_USE_PLATFORM_DEFAULT_STREAM#205helloworld922 wants to merge 6 commits into
Conversation
Changes to the cmake option for default stream
|
|
||
| option(CAMP_USE_PLATFORM_DEFAULT_STREAM "Whether CAMP should use the default device streams (On) or create separate streams (Off)" Off) | ||
|
|
||
| set(CAMP_USE_PLATFORM_DEFAULT_STREAM_NUM 0) |
There was a problem hiding this comment.
Nitpick: should set this to OFF and ON. If you use cmakedefine01, CMake will also set it to 0 and 1 respectively w/o having the additional @CAMP_USE_PLATFORM_DEFAULT_STREAM_NUM@. If that is really needed as opposed to just having it defined in preprocessor.
https://cmake.org/cmake/help/latest/command/configure_file.html#transformations
|
Used internally: https://github.com/llnl/camp/blob/56b194f3ea3d0e9d6e2f236de39a4886063e4968/include/camp/resource/hip.hpp#L187C5-L187C37 @helloworld922 , is the issue that we do not have a CMAKE variable to define it? |
|
Yes, the define is supposed to be user defined if they want to use default streams, but unless you properly forward the compiler define to all downstream dependencies you either end up with an ODR violation and/or not having the correct behavior of which stream kernels are being run on. This just makes sure that the define is baked into the installed RAJA headers so everyone picks up a consistent definition. |
| #cmakedefine CAMP_ENABLE_HIP | ||
| #cmakedefine CAMP_ENABLE_SYCL | ||
| #cmakedefine CAMP_WIN_STATIC_BUILD | ||
| #cmakedefine01 CAMP_USE_PLATFORM_DEFAULT_STREAM |
There was a problem hiding this comment.
I'd rather this be wrapped in an ifndef CAMP_USE_PLATFORM_DEFAULT_STREAM so we don't have a potential behavior change for projects that set this currently.
There was a problem hiding this comment.
I moved the check originally in defines.hpp to config.in.hpp
4 participants
No description provided.