[FEA] Unify incrementer_t test functors#9896
Conversation
d2af527 to
c52d64d
Compare
📝 WalkthroughSummary by CodeRabbit
WalkthroughChangesThe shared Incrementer functor unification
Assessment against linked issues
Suggested reviewers: Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b1f0e71d-51f3-40f5-b92e-2988c298c043
📒 Files selected for processing (6)
c2h/include/c2h/operator.cuhcub/test/catch2_test_device_bulk.cucub/test/catch2_test_device_for.cucub/test/catch2_test_device_for_copy.cucub/test/catch2_test_device_for_each_in_extents.cucub/test/catch2_test_device_for_each_in_layout.cu
| int* d_counts; | ||
|
|
||
| template <class OffsetT, class... Args> | ||
| __device__ void operator()(OffsetT i, Args...) | ||
| { | ||
| static_assert(cuda::std::is_void_v<ExpectedOffsetT> || cuda::std::is_same_v<ExpectedOffsetT, OffsetT>, | ||
| "ExpectedOffsetT and OffsetT must be the same type"); | ||
| atomicAdd(d_counts + i, 1); // Check if `i` was served more than once | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
important: Declare the immutable member const, and use the required device API annotation and noexcept on this non-throwing callable.
- int* d_counts;
+ int* const d_counts;
- __device__ void operator()(OffsetT i, Args...)
+ _CCCL_DEVICE_API void operator()(OffsetT i, Args...) const noexceptAs per coding guidelines, “All variables that are not modified must be declared const,” “Functions must be marked with _CCCL_*_API,” and non-throwing functions must be marked noexcept.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| int* d_counts; | |
| template <class OffsetT, class... Args> | |
| __device__ void operator()(OffsetT i, Args...) | |
| { | |
| static_assert(cuda::std::is_void_v<ExpectedOffsetT> || cuda::std::is_same_v<ExpectedOffsetT, OffsetT>, | |
| "ExpectedOffsetT and OffsetT must be the same type"); | |
| atomicAdd(d_counts + i, 1); // Check if `i` was served more than once | |
| } | |
| int* const d_counts; | |
| template <class OffsetT, class... Args> | |
| _CCCL_DEVICE_API void operator()(OffsetT i, Args...) const noexcept | |
| { | |
| static_assert(cuda::std::is_void_v<ExpectedOffsetT> || cuda::std::is_same_v<ExpectedOffsetT, OffsetT>, | |
| "ExpectedOffsetT and OffsetT must be the same type"); | |
| atomicAdd(d_counts + i, 1); // Check if `i` was served more than once | |
| } |
Source: Coding guidelines
1 participant
Description
Closes #9872
Centralizes the five duplicate
incrementer_ttest functors inc2h/include/c2h/operator.cuh. The shared functor accepts both one-argument and two-argument DeviceFor callbacks, while retaining the compile-time offset-type check used by the DeviceFor::Bulk test.Testing
pre-commit run --files c2h/include/c2h/operator.cuh cub/test/catch2_test_device_bulk.cu cub/test/catch2_test_device_for.cu cub/test/catch2_test_device_for_copy.cu cub/test/catch2_test_device_for_each_in_extents.cu cub/test/catch2_test_device_for_each_in_layout.cugit diff --checknvccor an NVIDIA GPU.Checklist