Repository navigation
Support fancy iterators in vectorized transform and port thrust::tabulate to it - #6012
Merged
Merged
Conversation
Contributor
|
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. |
bernhardmgruber
force-pushed
the
fancy_transform
branch
from
September 25, 2025 18:05
2208f23 to
d69fd95
Compare
miscco
reviewed
Sep 26, 2025
Comment on lines
+377
to
+379
| (int{sizeof(it_value_t<RandomAccessIteratorsIn>)} | ||
| * THRUST_NS_QUALIFIER::is_contiguous_iterator_v<RandomAccessIteratorsIn>) ..., | ||
| int{size_of<it_value_t<RandomAccessIteratorOut>>}); |
Contributor
There was a problem hiding this comment.
🙀 I believe those warrant a slightly more elaborate comment
Contributor
Author
There was a problem hiding this comment.
I think this needs a bigger refactoring in general
bernhardmgruber
force-pushed
the
fancy_transform
branch
2 times, most recently
from
September 26, 2025 10:50
3a9e091 to
512baf5
Compare
bernhardmgruber
marked this pull request as ready for review
September 26, 2025 11:05
NaderAlAwar
reviewed
Sep 26, 2025
This comment has been minimized.
This comment has been minimized.
bernhardmgruber
force-pushed
the
fancy_transform
branch
from
September 27, 2025 03:43
81f3ede to
8d22b69
Compare
This comment has been minimized.
This comment has been minimized.
miscco
approved these changes
Sep 29, 2025
Comment on lines
+249
to
+252
| constexpr int element_size = int{first_nonzero_value( | ||
| (sizeof(it_value_t<RandomAccessIteratorsIn>) | ||
| * THRUST_NS_QUALIFIER::is_contiguous_iterator_v<RandomAccessIteratorsIn>) ..., | ||
| size_of<output_t>)}; |
Contributor
There was a problem hiding this comment.
We should really pull that out into a function
Contributor
Author
There was a problem hiding this comment.
I am strongly considering to refactor the entire mess, so let's postpone any small fixes for now.
Comment on lines
+365
to
+368
| static constexpr bool can_memcpy_contiguous_inputs = | ||
| ((!THRUST_NS_QUALIFIER::is_contiguous_iterator_v<RandomAccessIteratorsIn> | ||
| || THRUST_NS_QUALIFIER::is_trivially_relocatable_v<it_value_t<RandomAccessIteratorsIn>>) | ||
| && ...); |
Contributor
There was a problem hiding this comment.
We should have a trait for that, I guess it will come up more often
bernhardmgruber
force-pushed
the
fancy_transform
branch
from
September 29, 2025 14:54
8d22b69 to
c2cb534
Compare
Contributor
🥳 CI Workflow Results🟩 Finished in 3h 48m: Pass: 100%/185 | Total: 8d 00h | Max: 3h 34m | Hits: 18%/189478See results here. |
bernhardmgruber
enabled auto-merge (squash)
September 29, 2025 19:14
NaderAlAwar
approved these changes
Sep 30, 2025
This was referenced May 19, 2026
1 task done
Naraenda
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 26, 2026
…ectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 cuda tabulate.h: took upstream (drop __tabulate::functor/parallel_for; implement via cuda_cub::transform_n over ::cuda::counting_iterator with ::cuda::proclaim_copyable_arguments(tabulate_op)); fancy-iterator support itself is in cub DeviceTransform. HIP: no change: performance-only (system/hip tabulate keeps its parallel_for path). benchmark/: bench/tabulate/basic.cu adapted to primbench: added sequence_benchmark (subalgo "sequence"), seg_size_t static_cast, output vectors use thrust::no_init, types widened to int8/16/32/64 (upstream integral_types); kept subalgo "basic" for the seg_size benchmark (upstream renamed it seg_size). test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
amd-jmahovsky
added a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 28, 2026
…sform over counting_iterator Related upstream commit: ad70011e990 Support fancy iterators in vectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Related local port commit: 291f437 The local port took upstream's cuda tabulate.h but left system/hip on its own parallel_for functor as "no change: performance-only". Bring system/hip in line with upstream's structure: - Drop __tabulate::functor; tabulate is a transform of _THRUST_LIBCXX::counting_iterator<iter_difference_t<Iterator>> into the output range, with granular libhipcxx includes as upstream. - hip has no transform_n overload (NVIDIA/cccl#5238 was cuda-only), so hip_rocprim::transform is called with [begin, begin + count). - tabulate_op is not wrapped in _THRUST_LIBCXX::proclaim_copyable_arguments: the proclamation only feeds cub::DeviceTransform, which hip's transform does not use, and the wrapper inherits from the functor, so it would reject function pointers and final classes for no benefit. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 29, 2026
…ectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 cuda tabulate.h: took upstream (drop __tabulate::functor/parallel_for; implement via cuda_cub::transform_n over ::cuda::counting_iterator with ::cuda::proclaim_copyable_arguments(tabulate_op)); fancy-iterator support itself is in cub DeviceTransform. HIP: no change: performance-only (system/hip tabulate keeps its parallel_for path). benchmark/: bench/tabulate/basic.cu adapted to primbench: added sequence_benchmark (subalgo "sequence"), seg_size_t static_cast, output vectors use thrust::no_init, types widened to int8/16/32/64 (upstream integral_types); kept subalgo "basic" for the seg_size benchmark (upstream renamed it seg_size). test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 29, 2026
…ectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 cuda tabulate.h: took upstream (drop __tabulate::functor/parallel_for; implement via cuda_cub::transform_n over ::cuda::counting_iterator with ::cuda::proclaim_copyable_arguments(tabulate_op)); fancy-iterator support itself is in cub DeviceTransform. HIP: no change: performance-only (system/hip tabulate keeps its parallel_for path). benchmark/: bench/tabulate/basic.cu adapted to primbench: added sequence_benchmark (subalgo "sequence"), seg_size_t static_cast, output vectors use thrust::no_init, types widened to int8/16/32/64 (upstream integral_types); kept subalgo "basic" for the seg_size benchmark (upstream renamed it seg_size). test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 29, 2026
…sform over counting_iterator Related upstream commit: ad70011e990 Support fancy iterators in vectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Related local port commit: 291f437 The local port took upstream's cuda tabulate.h but left system/hip on its own parallel_for functor as "no change: performance-only". Bring system/hip in line with upstream's structure: - Drop __tabulate::functor; tabulate is a transform of _THRUST_LIBCXX::counting_iterator<iter_difference_t<Iterator>> into the output range, with granular libhipcxx includes as upstream. - hip has no transform_n overload (NVIDIA/cccl#5238 was cuda-only), so hip_rocprim::transform is called with [begin, begin + count). - tabulate_op is not wrapped in _THRUST_LIBCXX::proclaim_copyable_arguments: the proclamation only feeds cub::DeviceTransform, which hip's transform does not use, and the wrapper inherits from the functor, so it would reject function pointers and final classes for no benefit. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Naraenda
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 30, 2026
…ectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 cuda tabulate.h: took upstream (drop __tabulate::functor/parallel_for; implement via cuda_cub::transform_n over ::cuda::counting_iterator with ::cuda::proclaim_copyable_arguments(tabulate_op)); fancy-iterator support itself is in cub DeviceTransform. HIP: no change: performance-only (system/hip tabulate keeps its parallel_for path). benchmark/: bench/tabulate/basic.cu adapted to primbench: added sequence_benchmark (subalgo "sequence"), seg_size_t static_cast, output vectors use thrust::no_init, types widened to int8/16/32/64 (upstream integral_types); kept subalgo "basic" for the seg_size benchmark (upstream renamed it seg_size). test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
Beanavil
pushed a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 30, 2026
…ectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 cuda tabulate.h: took upstream (drop __tabulate::functor/parallel_for; implement via cuda_cub::transform_n over ::cuda::counting_iterator with ::cuda::proclaim_copyable_arguments(tabulate_op)); fancy-iterator support itself is in cub DeviceTransform. HIP: no change: performance-only (system/hip tabulate keeps its parallel_for path). benchmark/: bench/tabulate/basic.cu adapted to primbench: added sequence_benchmark (subalgo "sequence"), seg_size_t static_cast, output vectors use thrust::no_init, types widened to int8/16/32/64 (upstream integral_types); kept subalgo "basic" for the seg_size benchmark (upstream renamed it seg_size). test/: no counterpart. testing/: no counterpart. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
amd-jmahovsky
added a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 30, 2026
…sform over counting_iterator Related upstream commit: ad70011e990 Support fancy iterators in vectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Related local port commit: 291f437 The local port took upstream's cuda tabulate.h but left system/hip on its own parallel_for functor as "no change: performance-only". Bring system/hip in line with upstream's structure: - Drop __tabulate::functor; tabulate is a transform of _THRUST_LIBCXX::counting_iterator<iter_difference_t<Iterator>> into the output range, with granular libhipcxx includes as upstream. - hip has no transform_n overload (NVIDIA/cccl#5238 was cuda-only), so hip_rocprim::transform is called with [begin, begin + count). - tabulate_op is not wrapped in _THRUST_LIBCXX::proclaim_copyable_arguments: the proclamation only feeds cub::DeviceTransform, which hip's transform does not use, and the wrapper inherits from the functor, so it would reject function pointers and final classes for no benefit. Not compiled, tests not run (per human) Co-Authored-By: Claude <noreply@anthropic.com>
amd-jmahovsky
added a commit
to ROCm/rocm-libraries
that referenced
this pull request
Oct 30, 2026
…sform_n with copyable arguments Related upstream commit: ad70011e990 Support fancy iterators in vectorized transform and port thrust::tabulate to it (NVIDIA/cccl#6012) Upstream: NVIDIA/cccl@ad70011 Related local port commit: 39a06d7 Related local parity commit: 4698506 4698506 turned hip tabulate into a transform over a counting_iterator, but called transform(begin, begin + count, ...) with the bare tabulate_op. Finish matching upstream in thrust/system/hip/detail/tabulate.h: call hip_rocprim::transform_n (added by the 76fc4572271 parity commit) on _THRUST_LIBCXX::counting_iterator<size_type>{} and count, and wrap tabulate_op in _THRUST_LIBCXX::proclaim_copyable_arguments. Includes follow upstream: __functional/address_stability.h and the _THRUST_STD iterator header replace __iterator/distance.h and __iterator/incrementable_traits.h. The counting_iterator include stays, since upstream only gets it through transform.h. Not compiled, tests not run Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
thrust::sequenceandthrust::tabulatebefore and after:B200
Fixes: #5808