-
Notifications
You must be signed in to change notification settings - Fork 67
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
Fix no-cuda build #484
Fix no-cuda build #484
Conversation
cpp/include/kvikio/utils.hpp
Outdated
#define KVIKIO_NVTX_FUNC_RANGE_2(msg, val) \ | ||
do { \ | ||
} while (0) | ||
#endif |
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.
Could shovel this into the shim, but because it only appears here that seems like overkill.
cpp/include/kvikio/utils.hpp
Outdated
@@ -300,6 +302,7 @@ struct libkvikio_domain { | |||
|
|||
// Macro overloads of KVIKIO_NVTX_FUNC_RANGE | |||
#define KVIKIO_NVTX_FUNC_RANGE_1() NVTX3_FUNC_RANGE_IN(libkvikio_domain) | |||
#ifdef KVIKIO_CUDA_FOUND |
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.
Why is this guard only needed for KVIKIO_NVTX_FUNC_RANGE_2
but not KVIKIO_NVTX_FUNC_RANGE_1
? The KVIKIO_NVTX_FUNC_RANGE_1
appears to call an NVTX macro but that wouldn't be defined if nvtx wasn't included above... right?
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.
Good catch, it worked because BASIC_NO_CUDA_EXAMPLE
never materialize KVIKIO_NVTX_FUNC_RANGE_1
.
I have included all of them in the ifdef guard now: 168dd0c
/merge |
Closes #482