Document ODR violations in KOKKOS_IF_ON_HOST/DEVICE constexpr usage#813
Document ODR violations in KOKKOS_IF_ON_HOST/DEVICE constexpr usage#813Copilot wants to merge 7 commits into
Conversation
… examples Co-authored-by: crtrott <9490481+crtrott@users.noreply.github.com>
… memory space Co-authored-by: crtrott <9490481+crtrott@users.noreply.github.com>
Co-authored-by: crtrott <9490481+crtrott@users.noreply.github.com>
| In this example, the ``Functor`` struct would have different sizes on the host | ||
| and device, causing serious memory corruption issues when passed between them. | ||
|
|
||
| **Problematic: Lambda captures with** ``constexpr`` **usage** |
There was a problem hiding this comment.
| **Problematic: Lambda captures with** ``constexpr`` **usage** | |
| **Problematic: Lambda capture dependency |
There was a problem hiding this comment.
Updated to "Lambda capture dependency" in commit c160bf9.
… specialization preference Co-authored-by: crtrott <9490481+crtrott@users.noreply.github.com>
| ``#ifdef __AVX2__`` in different translation units. | ||
|
|
||
| This is analogous to the following problematic pattern: | ||
|
|
||
| .. code-block:: cpp | ||
|
|
||
| // DO NOT DO THIS - ODR violation | ||
| static constexpr int foo() { | ||
| #ifdef __AVX2__ | ||
| return 4; | ||
| #else | ||
| return 2; | ||
| #endif | ||
| } | ||
|
|
||
| If you compile this code in two different translation units with different | ||
| compiler flags (one with AVX2 and one without) and then link them together, you | ||
| have an ODR violation because the same function has different definitions. |
There was a problem hiding this comment.
do we need this example? I think the next example makes it clear enough?
There was a problem hiding this comment.
I kinda do like to connect this to something not involving host/device compilation.
There was a problem hiding this comment.
but the explanation is about different compilation units with different flags...which if you know how some compilers do device code is helpful, but if you are oblivious to that, the question is: why shouldn't I do that?
There was a problem hiding this comment.
I also find the avx example very confusing.
There was a problem hiding this comment.
Per review let's not get into that compile flag mess here.
| and template instantiations. When ``KOKKOS_IF_ON_HOST`` and | ||
| ``KOKKOS_IF_ON_DEVICE`` are used in ``constexpr`` contexts, they cause the same | ||
| function or variable to have different compile-time values on the host versus | ||
| the device—similar to using architecture-specific preprocessor macros like |
There was a problem hiding this comment.
| the device—similar to using architecture-specific preprocessor macros like | |
| the device to using architecture-specific preprocessor macros like |
I get what it's getting to, but I feel like just the device is sufficient.
There was a problem hiding this comment.
I think a comma or a parenthesis should be added. It looks like the device-similar to ... was a kind of device (similar to ....
| )) | ||
| } | ||
|
|
||
| **Alternative 3 (Preferred): Use template specialization** |
There was a problem hiding this comment.
I think the Preferred method should be placed first as the Alternative 1.
| (constant expression). | ||
| These macros **must not** be used in a context that requires a ``constexpr`` | ||
| (constant expression). Using ``KOKKOS_IF_ON_HOST`` or ``KOKKOS_IF_ON_DEVICE`` | ||
| within ``constexpr`` functions or to initialize ``constexpr`` variables leads to |
There was a problem hiding this comment.
| within ``constexpr`` functions or to initialize ``constexpr`` variables leads to | |
| within ``constexpr`` functions or to initialize ``constexpr`` variables can lead to |
| These macros **must not** be used in a context that requires a ``constexpr`` | ||
| (constant expression). Using ``KOKKOS_IF_ON_HOST`` or ``KOKKOS_IF_ON_DEVICE`` | ||
| within ``constexpr`` functions or to initialize ``constexpr`` variables leads to | ||
| **One Definition Rule (ODR) violations** and undefined behavior. |
There was a problem hiding this comment.
| **One Definition Rule (ODR) violations** and undefined behavior. | |
| **One Definition Rule (ODR) violations** and cause undefined behavior. |
| ``#ifdef __AVX2__`` in different translation units. | ||
|
|
||
| This is analogous to the following problematic pattern: | ||
|
|
||
| .. code-block:: cpp | ||
|
|
||
| // DO NOT DO THIS - ODR violation | ||
| static constexpr int foo() { | ||
| #ifdef __AVX2__ | ||
| return 4; | ||
| #else | ||
| return 2; | ||
| #endif | ||
| } | ||
|
|
||
| If you compile this code in two different translation units with different | ||
| compiler flags (one with AVX2 and one without) and then link them together, you | ||
| have an ODR violation because the same function has different definitions. |
There was a problem hiding this comment.
Per review let's not get into that compile flag mess here.
Signed-off-by: Christian Trott <crtrott@sandia.gov>
Signed-off-by: Christian Trott <crtrott@sandia.gov>
| **Problematic: Lambda capture dependency** | ||
|
|
||
| .. code-block:: cpp | ||
|
|
||
| // DO NOT DO THIS - causes ODR violation | ||
| void foo() { | ||
| int a = 0; | ||
| double b = 1.0; | ||
| auto lambda = KOKKOS_LAMBDA(int) { | ||
| KOKKOS_IF_ON_HOST((printf("%i\n", a);)) // Captures 'a' | ||
| KOKKOS_IF_ON_DEVICE((printf("%lf\n", b);)) // Captures 'b' | ||
| }; | ||
| // Lambda has different size on host vs device due to different captures | ||
| } | ||
|
|
||
| The lambda object has different sizes on host and device because of the | ||
| different captures, violating the ODR. |
There was a problem hiding this comment.
So this problematic example never gets resolved, unlike the one above? If so, I think this example should be first, then the one that gets resolved should be second and it should be clear that the "Correct Alternatives" section is a subsection of that example.
Extend constexpr Context Documentation for KOKKOS_IF_ON_HOST/DEVICE ✅
Summary of Changes:
Extended the "constexpr Context" section in the documentation of
KOKKOS_IF_ON_HOSTandKOKKOS_IF_ON_DEVICEmacros with comprehensive explanation of ODR violations based on developer discussions.Completed Work:
Documentation Enhancements:
#ifdef __AVX2__preprocessor usage across translation unitsBuild Verification:
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.