Add cmake option for CAMP_USE_PLATFORM_DEFAULT_STREAM#205
Add cmake option for CAMP_USE_PLATFORM_DEFAULT_STREAM#205helloworld922 wants to merge 11 commits into
Conversation
Changes to the cmake option for default stream
|
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. |
| #else | ||
| #cmakedefine01 CAMP_USE_PLATFORM_DEFAULT_STREAM_INTERNAL_CHECK | ||
| #if CAMP_USE_PLATFORM_DEFAULT_STREAM != CAMP_USE_PLATFORM_DEFAULT_STREAM_INTERNAL_CHECK | ||
| #warning "CAMP_USE_PLATFORM_DEFAULT_STREAM mismatch, potential ODR violation" |
There was a problem hiding this comment.
@helloworld922 If you no longer will need to define CAMP_USE_PLATFORM_DEFAULT_STREAM from outside of this header we were thinking about disallowing that to avoid ODR violations.
There was a problem hiding this comment.
I think that would be ideal. I moved the check to a part of config.in.hpp which should only be defined once. It still supports users having manually defined CAMP_USE_PLATFORM_DEFAULT_STREAM for backwards compatibility, but it must match how RAJA is configured to be built.
Alternatively, I could just remove this and force all users to not have CAMP_USE_PLATFORM_DEFAULT_STREAM externally defined at all.
There was a problem hiding this comment.
I like the idea of a hard error if users have CAMP_USE_PLATFORM_DEFAULT_STREAM externally defined:
#if defined(CAMP_USE_PLATFORM_DEFAULT_STREAM)
#error "Manually defining CAMP_USE_PLATFORM_DEFAULT_STREAM is not allowed because of the potential for ODR violations. CAMP now supports a CMake configuration option that should be used instead."
#endif
#cmakedefine01 CAMP_USE_PLATFORM_DEFAULT_STREAM
| #define CAMP_DLL_EXPORT | ||
| #endif | ||
|
|
||
| #ifndef CAMP_CONFIG_HPP |
There was a problem hiding this comment.
@trws, @MrBurmark should this header guard be at the beginning of the file or after the CAMP_CONFIG_OVERRIDE section?
There was a problem hiding this comment.
Previously there was no header guard at all, so it would make sense to me to add one around everything. @trws was there a specific reason not to guard this header?
There was a problem hiding this comment.
I assume that even if you are using the override you can only want to config camp once per translation unit.
7 participants
No description provided.