Repository navigation
Refactor iterator usage in thrust/cuda find() - #6019
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. |
| // FIXME(bgruber): we should also be able to use transform_iterator here, but it makes nvc++ hang. See: | ||
| // https://github.com/NVIDIA/cccl/issues/3594. The problem does not occur with nvcc, so we could not add a test :/ |
There was a problem hiding this comment.
nvc++ is fine with this code
|
|
||
| result_type result = reduce( | ||
| const result_type result = reduce( | ||
| policy, interval_begin, interval_end, result_type(false, interval_end - begin), __find_if::functor<result_type>()); |
There was a problem hiding this comment.
I get an error here at interval_end - begin because zip_iterator::operator- is disabled because _Constraints::__all_sized_sentinel fails because the transform iterator above is not default constructible (the predicate isn't default constructible). The latter is fine, so I wonder why the constraint on zip_iterator::operator- is needed.
There was a problem hiding this comment.
This is from ::std::ranges::zip_view
It kind of also boils down to the iterator concepts, because we want a forward iterator to be regular which requires default constructibility
There was a problem hiding this comment.
That's a problem now, because if we replace any thrust zip iterator by a cuda one, it can be a source breaking change for a user.
There was a problem hiding this comment.
Discussed with @miscco offline: he will relax the constraint on operator-
There was a problem hiding this comment.
Put this in #6036 because we had to fix zip_iterator for NVHPC anyway
56595e7 to
90bd55a
Compare
This comment has been minimized.
This comment has been minimized.
90bd55a to
d6fdc19
Compare
This comment has been minimized.
This comment has been minimized.
87c0680 to
dd654b8
Compare
This comment has been minimized.
This comment has been minimized.
b79f5d3 to
02e4adb
Compare
This comment has been minimized.
This comment has been minimized.
705995e to
7b9e329
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
7b9e329 to
d285f7b
Compare
This comment has been minimized.
This comment has been minimized.
55432b6 to
e41a347
Compare
🥳 CI Workflow Results🟩 Finished in 4h 58m: Pass: 100%/118 | Total: 7d 17h | Max: 4h 57m | Hits: 33%/166765See results here. |
…hrust/cuda find() (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@1c3c7dd HIP: system/hip/detail/mismatch.h transform_pair_of_input_iterators_t got the iterator correctness fixes (defaulted default ctor, prefix operator++ returns self_t&, operator<); the find() zip/transform/counting_iterator rewrite is performance-only, so HIP find.h is unchanged. test/: no counterpart (no test change upstream beyond CUDA-only file). testing/: cuda/mismatch.cu unused-parameter cleanup taken. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…ind/mismatch Upstream: NVIDIA/cccl@1c3c7ddee12 (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@8ca3dc3 (NVIDIA/cccl#6018) HIP previously took no change for these (performance-only). Bring system/hip closer to upstream: - find.h: find_if_n unwraps contiguous iterators and builds the zip/counting iterator with auto + variadic thrust::make_zip_iterator. transform_input_iterator_t now calls op(*input) directly instead of materializing the value_type (same semantics as ::cuda::transform_iterator; avoids NVIDIA/cccl#3591); prefix operator++ returns self_t&. The dedicated iterator is kept because thrust::transform_iterator still converts to the value_type and ::cuda::transform_iterator needs libhipcxx. - mismatch.h: drop transform_pair_of_input_iterators_t and enable the zip_iterator + zip_function implementation (thrust:: versions, so it also works without libhipcxx). Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…tors in hip find/mismatch Related upstream commit: 8ca3dc31e90 Refactor thrust::mismatch to use CUDA iterators (NVIDIA/cccl#6018) Related upstream commit: 1c3c7ddee12 Refactor iterator usage in thrust/cuda find() (NVIDIA/cccl#6019) Related local port commit: e46dc36 e46dc36 brought the upstream find/mismatch rewrite to system/hip using thrust:: iterators, plus a local transform_input_iterator_t because thrust::transform_iterator materializes the value_type before calling the predicate (NVIDIA/cccl#3591). libhipcxx is now mandatory, so hip now uses the same libcu++ iterators as upstream's system/cuda: - find.h: find_if_n zips _THRUST_LIBCXX::make_transform_iterator( try_unwrap_contiguous_iterator(first), predicate) with _THRUST_LIBCXX::counting_iterator<Size>(0) and uses a _THRUST_STD::tuple<bool, Size> result type; __find_if:: transform_input_iterator_t and the dead !_THRUST_HAS_DEVICE_SYSTEM_STD include block are removed; upstream granular includes are taken (<thrust/functional.h> kept explicitly for the placeholders in find). - mismatch.h: _THRUST_LIBCXX::make_zip_iterator and _THRUST_LIBCXX::zip_function(binary_pred), with upstream's granular includes instead of thrust/{distance,pair,zip_function}.h and thrust/iterator/zip_iterator.h. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…hrust/cuda find() (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@1c3c7dd HIP: system/hip/detail/mismatch.h transform_pair_of_input_iterators_t got the iterator correctness fixes (defaulted default ctor, prefix operator++ returns self_t&, operator<); the find() zip/transform/counting_iterator rewrite is performance-only, so HIP find.h is unchanged. test/: no counterpart (no test change upstream beyond CUDA-only file). testing/: cuda/mismatch.cu unused-parameter cleanup taken. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…hrust/cuda find() (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@1c3c7dd HIP: system/hip/detail/mismatch.h transform_pair_of_input_iterators_t got the iterator correctness fixes (defaulted default ctor, prefix operator++ returns self_t&, operator<); the find() zip/transform/counting_iterator rewrite is performance-only, so HIP find.h is unchanged. test/: no counterpart (no test change upstream beyond CUDA-only file). testing/: cuda/mismatch.cu unused-parameter cleanup taken. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…ind/mismatch Upstream: NVIDIA/cccl@1c3c7ddee12 (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@8ca3dc3 (NVIDIA/cccl#6018) HIP previously took no change for these (performance-only). Bring system/hip closer to upstream: - find.h: find_if_n unwraps contiguous iterators and builds the zip/counting iterator with auto + variadic thrust::make_zip_iterator. transform_input_iterator_t now calls op(*input) directly instead of materializing the value_type (same semantics as ::cuda::transform_iterator; avoids NVIDIA/cccl#3591); prefix operator++ returns self_t&. The dedicated iterator is kept because thrust::transform_iterator still converts to the value_type and ::cuda::transform_iterator needs libhipcxx. - mismatch.h: drop transform_pair_of_input_iterators_t and enable the zip_iterator + zip_function implementation (thrust:: versions, so it also works without libhipcxx). Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…tors in hip find/mismatch Related upstream commit: 8ca3dc31e90 Refactor thrust::mismatch to use CUDA iterators (NVIDIA/cccl#6018) Related upstream commit: 1c3c7ddee12 Refactor iterator usage in thrust/cuda find() (NVIDIA/cccl#6019) Related local port commit: e46dc36 e46dc36 brought the upstream find/mismatch rewrite to system/hip using thrust:: iterators, plus a local transform_input_iterator_t because thrust::transform_iterator materializes the value_type before calling the predicate (NVIDIA/cccl#3591). libhipcxx is now mandatory, so hip now uses the same libcu++ iterators as upstream's system/cuda: - find.h: find_if_n zips _THRUST_LIBCXX::make_transform_iterator( try_unwrap_contiguous_iterator(first), predicate) with _THRUST_LIBCXX::counting_iterator<Size>(0) and uses a _THRUST_STD::tuple<bool, Size> result type; __find_if:: transform_input_iterator_t and the dead !_THRUST_HAS_DEVICE_SYSTEM_STD include block are removed; upstream granular includes are taken (<thrust/functional.h> kept explicitly for the placeholders in find). - mismatch.h: _THRUST_LIBCXX::make_zip_iterator and _THRUST_LIBCXX::zip_function(binary_pred), with upstream's granular includes instead of thrust/{distance,pair,zip_function}.h and thrust/iterator/zip_iterator.h. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…hrust/cuda find() (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@1c3c7dd HIP: system/hip/detail/mismatch.h transform_pair_of_input_iterators_t got the iterator correctness fixes (defaulted default ctor, prefix operator++ returns self_t&, operator<); the find() zip/transform/counting_iterator rewrite is performance-only, so HIP find.h is unchanged. test/: no counterpart (no test change upstream beyond CUDA-only file). testing/: cuda/mismatch.cu unused-parameter cleanup taken. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…hrust/cuda find() (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@1c3c7dd HIP: system/hip/detail/mismatch.h transform_pair_of_input_iterators_t got the iterator correctness fixes (defaulted default ctor, prefix operator++ returns self_t&, operator<); the find() zip/transform/counting_iterator rewrite is performance-only, so HIP find.h is unchanged. test/: no counterpart (no test change upstream beyond CUDA-only file). testing/: cuda/mismatch.cu unused-parameter cleanup taken. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…ind/mismatch Upstream: NVIDIA/cccl@1c3c7ddee12 (NVIDIA/cccl#6019) Upstream: NVIDIA/cccl@8ca3dc3 (NVIDIA/cccl#6018) HIP previously took no change for these (performance-only). Bring system/hip closer to upstream: - find.h: find_if_n unwraps contiguous iterators and builds the zip/counting iterator with auto + variadic thrust::make_zip_iterator. transform_input_iterator_t now calls op(*input) directly instead of materializing the value_type (same semantics as ::cuda::transform_iterator; avoids NVIDIA/cccl#3591); prefix operator++ returns self_t&. The dedicated iterator is kept because thrust::transform_iterator still converts to the value_type and ::cuda::transform_iterator needs libhipcxx. - mismatch.h: drop transform_pair_of_input_iterators_t and enable the zip_iterator + zip_function implementation (thrust:: versions, so it also works without libhipcxx). Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
…tors in hip find/mismatch Related upstream commit: 8ca3dc31e90 Refactor thrust::mismatch to use CUDA iterators (NVIDIA/cccl#6018) Related upstream commit: 1c3c7ddee12 Refactor iterator usage in thrust/cuda find() (NVIDIA/cccl#6019) Related local port commit: e46dc36 e46dc36 brought the upstream find/mismatch rewrite to system/hip using thrust:: iterators, plus a local transform_input_iterator_t because thrust::transform_iterator materializes the value_type before calling the predicate (NVIDIA/cccl#3591). libhipcxx is now mandatory, so hip now uses the same libcu++ iterators as upstream's system/cuda: - find.h: find_if_n zips _THRUST_LIBCXX::make_transform_iterator( try_unwrap_contiguous_iterator(first), predicate) with _THRUST_LIBCXX::counting_iterator<Size>(0) and uses a _THRUST_STD::tuple<bool, Size> result type; __find_if:: transform_input_iterator_t and the dead !_THRUST_HAS_DEVICE_SYSTEM_STD include block are removed; upstream granular includes are taken (<thrust/functional.h> kept explicitly for the placeholders in find). - mismatch.h: _THRUST_LIBCXX::make_zip_iterator and _THRUST_LIBCXX::zip_function(binary_pred), with upstream's granular includes instead of thrust/{distance,pair,zip_function}.h and thrust/iterator/zip_iterator.h. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Merge first:
zip_iterator#6036