-
Notifications
You must be signed in to change notification settings - Fork 163
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
Enable span to work with contiguous std containers in C++17 #2613
Conversation
🟨 CI finished in 1h 55m: Pass: 90%/366 | Total: 3d 21h | Avg: 15m 22s | Max: 1h 26m | Hits: 32%/19395
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 366)
# | Runner |
---|---|
298 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
The concept seems to be too relaxed now, it fails some checks |
Oh yeah the check is that something with only the random_access_iterator_tag does not satisfy contiguous_iterator However, |
ca506a4
to
2cadd3c
Compare
🟨 CI finished in 1h 08m: Pass: 98%/366 | Total: 1d 20h | Avg: 7m 13s | Max: 49m 13s | Hits: 86%/22395
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 366)
# | Runner |
---|---|
298 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
2cadd3c
to
2374de2
Compare
🟨 CI finished in 2h 18m: Pass: 98%/366 | Total: 3d 06h | Avg: 12m 53s | Max: 1h 26m | Hits: 35%/22395
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 366)
# | Runner |
---|---|
298 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
@@ -451,7 +451,6 @@ _LIBCUDACXX_CONCEPT_FRAGMENT( | |||
__contiguous_iterator_, | |||
requires(const _Ip& __i)( | |||
requires(random_access_iterator<_Ip>), | |||
requires(derived_from<_ITER_CONCEPT<_Ip>, 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.
can we scope this change to just c++17?
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.
That is C++17 only, because C++20 uses plain concepts
I still have some trouble understanding the results of this change. I get that it will enable to construct But is it possible that the |
2374de2
to
4703ac8
Compare
🟨 CI finished in 2h 41m: Pass: 98%/394 | Total: 5d 22h | Avg: 21m 41s | Max: 1h 30m | Hits: 32%/21335
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 394)
# | Runner |
---|---|
326 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
I have been pondering this more and I believe the "right" fix is to rather change I dont see a way of making it correct otherwise |
contiguous_iterator_tag
in C++17 as that is not satisfied by any standard containerdecltype(_CUDA_VSTD::data(_CUDA_VSTD::declval<_Container&>())), | ||
decltype(_CUDA_VSTD::size(_CUDA_VSTD::declval<_Container&>())), |
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.
That is an actual fix, because we want to avoid pulling in anything via ADL
cff076e
to
6b279a3
Compare
🟩 CI finished in 1h 06m: Pass: 100%/394 | Total: 2d 12h | Avg: 9m 11s | Max: 46m 44s | Hits: 91%/25785
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 394)
# | Runner |
---|---|
326 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
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.
Thanks a lot for looking into it!
Co-authored-by: pciolkosz <[email protected]>
🟨 CI finished in 2h 11m: Pass: 99%/394 | Total: 1d 23h | Avg: 7m 13s | Max: 39m 12s | Hits: 91%/25785
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 394)
# | Runner |
---|---|
326 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
🟩 CI finished in 3h 19m: Pass: 100%/394 | Total: 1d 23h | Avg: 7m 12s | Max: 39m 12s | Hits: 91%/25785
|
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
CUB | |
Thrust | |
CUDA Experimental | |
pycuda | |
CCCL C Parallel Library |
Modifications in project or dependencies?
Project | |
---|---|
CCCL Infrastructure | |
+/- | libcu++ |
+/- | CUB |
+/- | Thrust |
+/- | CUDA Experimental |
+/- | pycuda |
+/- | CCCL C Parallel Library |
🏃 Runner counts (total jobs: 394)
# | Runner |
---|---|
326 | linux-amd64-cpu16 |
28 | linux-arm64-cpu16 |
25 | linux-amd64-gpu-v100-latest-1 |
15 | windows-amd64-cpu16 |
) * Do not require `contiguous_iterator_tag` in C++17 as that is not satisfied by any standard container * Do not require ranges for spans C++17 constructors --------- Co-authored-by: Eric Niebler <[email protected]> Co-authored-by: pciolkosz <[email protected]>
No description provided.