-
Notifications
You must be signed in to change notification settings - Fork 170
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
help the ranges concepts recognize standard contiguous iterators in c++14/17 #3202
base: main
Are you sure you want to change the base?
Conversation
d527d62
to
21907a4
Compare
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 really did not wanted to do that, but its most likely the easiest solution.
Can you please then fix our span
to always use the proper range constructor and not the pre-ranges hack?
_LIBCUDACXX_HIDE_FROM_ABI auto __iter_concept_fn(_Iter, __priority_tag<3>) -> contiguous_iterator_tag; | ||
_CCCL_TEMPLATE(class _Iter) | ||
_CCCL_REQUIRES(_IsSame<_Iter, class _Iter::_Span_iterator>::value) | ||
_LIBCUDACXX_HIDE_FROM_ABI auto __iter_concept_fn(_Iter, __priority_tag<3>) -> contiguous_iterator_tag; |
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.
You can probably do this more cleanly in MSVC by detecting if our iterator unwrapping machinery produces a pointer from a given iterator type. std::is_pointer_v<std::_Unwrapped_t<_Iter>>
is true
for the STL's contiguous iterators, and for gsl:span
's iterators as well. (I didn't tell you to take this dependency on STL internals, I was never 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.
hrm. we need to support older versions of msvc. when did this change go in?
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 don't recall precisely when we defined the alias _Unwrapped_t
, but is_pointer_v<decltype(::std::_Get_unwrapped(::std::declval<_Iter>()))>
isn't much more to type and should work back to at least VS2015. CE only goes back to 16.0 (https://www.godbolt.org/z/h4Wa1YvEc), and that works. I don't have a readily available VS2015 to test. Is that old enough?
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.
Works fine with VS2017 v15.9, not at all with VS2015.
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.
We only support 2017 gladly
🟩 CI finished in 2h 39m: Pass: 100%/170 | Total: 3d 18h | Avg: 31m 55s | Max: 1h 19m | Hits: 33%/22510
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
+/- | CUDA Experimental |
python | |
CCCL C Parallel Library | |
Catch2Helper |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | python |
+/- | CCCL C Parallel Library |
+/- | Catch2Helper |
🏃 Runner counts (total jobs: 170)
# | Runner |
---|---|
125 | linux-amd64-cpu16 |
19 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
10 | linux-arm64-cpu16 |
1 | linux-amd64-gpu-h100-latest-1-testing |
i started down this rabbit hole, but the range stuff is only defined in C++17, and (for now) |
_CCCL_TEMPLATE(class _Iter, class _Ty, class _Range) | ||
_CCCL_REQUIRES(_IsSame<_Iter, ::__gnu_cxx::__normal_iterator<_Ty*, _Range>>::value) | ||
_LIBCUDACXX_HIDE_FROM_ABI auto | ||
__iter_concept_fn(::__gnu_cxx::__normal_iterator<_Ty*, _Range>, __priority_tag<3>) -> contiguous_iterator_tag; | ||
#endif | ||
#if defined(_LIBCPP_VERSION) | ||
# elif defined(_LIBCPP_VERSION) |
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 am sorry, but my comment was erroneous.
We always pull in libstdc++ through some nvcc machinery, so we need to make this a separate branch
Description
In range-v3, @CaseyCarter and I found a sneaky way to "detect" the contiguous iterators of the 3 major stdlib implementations: libstdc++, libc++, and msvc's stdlib. This PR ports the implementation over so that the following standard containers are now recognized as contiguous:
std::vector
std::array
std::string
std::string_view
this PR also removes a hack in cudax that was only necessary because the std:: containers weren't considered contiguous in c++17.
Checklist