Skip to content
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

Ensure cuda/stream_ref can be used without a CUDA compiler #3370

Closed
wants to merge 3 commits into from

Conversation

caugonnet
Copy link
Contributor

Description

Implementing ::cuda::__throw_cuda_error without a CUDA compiler has some challenging, so instead we make it possible to include cuda/stream_ref without it

Checklist

  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

Copy link

copy-pr-bot bot commented Jan 13, 2025

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@caugonnet
Copy link
Contributor Author

/ok to test

@caugonnet
Copy link
Contributor Author

/ok to test

throw ::std::runtime_error("Failed to query stream.");
# else
::std::terminate();
# else
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a typo. This change is needed to make builds succeed for me.

Suggested change
# else
# endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm puzzled why this wasn't caught in CI, we are indeed not testing non-CUDA compilers at all .... Maybe we are not reaching that path anymore due to other PR now avoiding to include this header earlier if we are not in a CUDA compiler ?

@@ -9,7 +9,7 @@
//===----------------------------------------------------------------------===//

#ifndef _CUDA_STREAM_REF
#define _CUDA_STREAM_REF
# define _CUDA_STREAM_REF
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to indent all these macros? I would prefer to leave those unchanged in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that some effect of a recent clang-format update ?

@bdice
Copy link
Contributor

bdice commented Jan 30, 2025

I think this issue is fixed by #3472. @caugonnet Feel free to close if you agree.

@caugonnet caugonnet closed this Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants