-
Notifications
You must be signed in to change notification settings - Fork 283
Add host standard library detection #6244
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
Conversation
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
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.
Looks great, recapitulating the internal discussion for posterity
/ok to test e1bcf4b |
/ok to test af71337 |
/ok to test 0e45c13 |
/ok to test 81588a9 |
/ok to test 89b181a |
/ok to test e17232f |
e17232f
to
081bb70
Compare
081bb70
to
dd0b986
Compare
template <class _CharT, class _Traits, class _Tp = float> | ||
::std::basic_istream<_CharT, _Traits>& | ||
operator>>(::std::basic_istream<_CharT, _Traits>& __is, complex<__nv_bfloat16>& __x) | ||
{ | ||
::std::complex<float> __temp; | ||
::std::complex<_Tp> __temp; |
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.
This is necessary to prevent errors about the type not being defined, but only forward declared
template <class _Up = value_type> | ||
_CCCL_HOST operator ::std::complex<_Up>() const |
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.
Again..
{ | ||
# if __cpp_lib_string_view >= 201606L | ||
return __os << ::std::basic_string_view<_CharT>{__str}; | ||
return __os << ::std::basic_string_view<_CharT, ::std::char_traits<_CharT>>{__str}; |
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.
Because we only forward declare the types, we don't have access to default template parameters, so we have to pass all of the template paramters here.
template <class _CharT, class _Traits, class _Tp = float> | ||
::std::basic_ostream<_CharT, _Traits>& | ||
operator<<(::std::basic_ostream<_CharT, _Traits>& __os, const complex<__nv_bfloat16>& __x) | ||
{ | ||
return __os << complex<float>{__x}; | ||
return __os << complex<_Tp>{__x}; | ||
} |
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.
This one should not be necessary because we are converting to a cuda::std::complex
which is defined
operator>>(::std::basic_istream<_CharT, _Traits>& __is, complex<__nv_bfloat16>& __x) | ||
{ | ||
::std::complex<float> __temp; | ||
::std::complex<_Tp> __temp; |
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.
I believe we could get away with the extended floating point types always casting to ::cuda::std::complex<float>
and only do the smart thing for that
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
9231bee
to
ce1d9df
Compare
ce1d9df
to
02ff513
Compare
# endif | ||
# endif // defined(_GLIBCXX_VERSION) || defined(_LIBCPP_VERSION) || defined(_MSVC_STL_VERSION) | ||
#endif // defined(__cplusplus) | ||
// todo: re-enable std builtins |
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.
They were not working anyway, so I'd like to leave this for a different PR
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.
I would love to know what is not woorking. Generally I would just replace the host STL detection and leave the rest in
} | ||
}; | ||
|
||
# if !defined(_LIBCUDACXX_HAS_NO_LOCALIZATION) && !_CCCL_COMPILER(NVRTC) |
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.
Is never defined
This comment has been minimized.
This comment has been minimized.
# endif | ||
# endif // defined(_GLIBCXX_VERSION) || defined(_LIBCPP_VERSION) || defined(_MSVC_STL_VERSION) | ||
#endif // defined(__cplusplus) | ||
// todo: re-enable std builtins |
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.
I would love to know what is not woorking. Generally I would just replace the host STL detection and leave the rest in
🥳 CI Workflow Results🟩 Finished in 6h 53m: Pass: 100%/127 | Total: 5d 18h | Max: 3h 20m | Hits: 42%/306565See results here. |
@ericniebler implemented some host standard library detection for using intrinsics for some simple functions such as
move
orforward
.I've improved the design a bit further, moved it to a separate header and added host standard library namespaces. The idea is that we could forward declare things like
std::string
instead of including the whole header to reduce compilation times.The user would be required to include those headers if he wants to use APIs including these types.
Btw.
_GLIBCXX_VERSION
doesn't exist, I've changed it to__GLIBCXX__
.