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
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions libcudacxx/include/cuda/stream_ref
Original file line number Diff line number Diff line change
Expand Up @@ -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 ?


/*
stream_ref synopsis
Expand Down Expand Up @@ -38,22 +38,22 @@ private:
} // cuda
*/

#include <cuda_runtime_api.h>
# include <cuda_runtime_api.h>
// cuda_runtime_api needs to come first

#include <cuda/std/detail/__config>
# include <cuda/std/detail/__config>

#if defined(_CCCL_IMPLICIT_SYSTEM_HEADER_GCC)
# pragma GCC system_header
#elif defined(_CCCL_IMPLICIT_SYSTEM_HEADER_CLANG)
# pragma clang system_header
#elif defined(_CCCL_IMPLICIT_SYSTEM_HEADER_MSVC)
# pragma system_header
#endif // no system header
# if defined(_CCCL_IMPLICIT_SYSTEM_HEADER_GCC)
# pragma GCC system_header
# elif defined(_CCCL_IMPLICIT_SYSTEM_HEADER_CLANG)
# pragma clang system_header
# elif defined(_CCCL_IMPLICIT_SYSTEM_HEADER_MSVC)
# pragma system_header
# endif // no system header

#include <cuda/std/__cuda/api_wrapper.h>
#include <cuda/std/__exception/cuda_error.h>
#include <cuda/std/cstddef>
# include <cuda/std/__cuda/api_wrapper.h>
# include <cuda/std/__exception/cuda_error.h>
# include <cuda/std/cstddef>

_LIBCUDACXX_BEGIN_NAMESPACE_CUDA

Expand Down Expand Up @@ -164,7 +164,15 @@ public:
break;
default:
::cudaGetLastError(); // Clear CUDA error state
# ifdef _CCCL_HAS_CUDA_COMPILER
::cuda::__throw_cuda_error(__result, "Failed to query stream.");
# else
# ifndef _CCCL_NO_EXCEPTIONS
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 ?

# endif
}
return true;
}
Expand All @@ -186,4 +194,4 @@ public:

_LIBCUDACXX_END_NAMESPACE_CUDA

#endif //_CUDA_STREAM_REF
# endif //_CUDA_STREAM_REF
Loading