[rocPRIM][Code Coverage][Cherry Pick] Increase code coverage for rocPRIM#1036
Merged
vamovsik merged 4 commits intoAug 8, 2025
Conversation
In this commit we have added test coverage, made some bug fixes and added a wrapper function for a common pattern in the tests. This `test_kernel_wrapper` function reduces the amount of code used. In 0318f42 we have added coverage of the `warp_scan_shuffle` by forcing dpp off in cmake on the `warp_scan` test. In 30d54c1 we have added the missing test cases for the thread_algos in the thread directory. (Except for thread_operators, these functions are mostly duplicates probably needs to be cleanly moved/deprecated). We also found a bug in `thread_reduce` with the tests and fixed this. In 48780ff 37aa542 05a9e12 f5f1dbe 343e275 8007fe6 we have added unit tests for the files in the types directory. In 40ca3e2 we have introduced the `test_kernel_wrapper` and added the missing `PartitionTwoWayFlag` test. In 7a4e7ba unit tests for the rocprim::tuple type are added. In 29eac9a we have added extra test coverage by introducing a new type that will go into the untested path. We have also added this type to the benchmark to show [this specialization](29eac9a#diff-22a70b2ad081732e222004ded43ce0db7145ff196f7b723289425dcb5b6c732dR228) is still needed. We also changed the `std::is_integral` to `rocprim::is_integral` to not include `(u)int128_t` for this specialization, this does not impact performance. The specialization enable_if was also slightly changed to make it clearer which path is chosen (does not make a difference in actual executed code). Also custom config and a iterator was added to the tests of merge_sort. In f5614d5 test coverage was added for the device_scan_common.hpp file. In 1d2a40a test coverage was increased by actually using `const fixed_array` and including a `Level` type which goes into the base path of sample_to_bin_even struct. We also added an iterator as a type to the test. Also the new test wrapper was used. In c518f42 test coverage was increased for the iterators, a lot of the operators where missing. Also some cleaning up of the tests was done including the wrapper when possible. There was also a bug found for the comperator in `arg_index_iterator` and `texture_cache_iterator`, which was also fixed. The `->` operator is not tested for `test_texture_cache`, `arg_index_iterator`, `transform_iterator` and `zip_iterator`. They currently do not seem to work, I am working on a possible fix but will be added in a later PR. --------- Co-authored-by: Saiyang Zhang <saiyang@streamhpc.com> Co-authored-by: Nara Prasetya <nara@streamhpc.com> Co-authored-by: Cenxuan Tian <cenxuan@streamhpc.com>
xiaohuguo2023
pushed a commit
to xiaohuguo2023/rocm-libraries
that referenced
this pull request
Aug 3, 2025
spolifroni-amd
approved these changes
Aug 5, 2025
stanleytsang-amd
approved these changes
Aug 5, 2025
…-increase-code-coverage-cherry-pick-7.0
…-increase-code-coverage-cherry-pick-7.0
assistant-librarian Bot
pushed a commit
to ROCm/rocPRIM
that referenced
this pull request
Aug 8, 2025
[rocPRIM][Code Coverage][Cherry Pick] Increase code coverage for rocPRIM (#1036) In this commit we have added test coverage, made some bug fixes and added a wrapper function for a common pattern in the tests. This `test_kernel_wrapper` function reduces the amount of code used. In ROCm/rocm-libraries@0318f42 we have added coverage of the `warp_scan_shuffle` by forcing dpp off in cmake on the `warp_scan` test. In ROCm/rocm-libraries@30d54c1 we have added the missing test cases for the thread_algos in the thread directory. (Except for thread_operators, these functions are mostly duplicates probably needs to be cleanly moved/deprecated). We also found a bug in `thread_reduce` with the tests and fixed this. In ROCm/rocm-libraries@48780ff ROCm/rocm-libraries@37aa542 ROCm/rocm-libraries@05a9e12 ROCm/rocm-libraries@f5f1dbe ROCm/rocm-libraries@343e275 ROCm/rocm-libraries@8007fe6 we have added unit tests for the files in the types directory. In ROCm/rocm-libraries@40ca3e2 we have introduced the `test_kernel_wrapper` and added the missing `PartitionTwoWayFlag` test. In ROCm/rocm-libraries@7a4e7ba unit tests for the rocprim::tuple type are added. In ROCm/rocm-libraries@29eac9a we have added extra test coverage by introducing a new type that will go into the untested path. We have also added this type to the benchmark to show [this specialization](ROCm/rocm-libraries@29eac9a#diff-22a70b2ad081732e222004ded43ce0db7145ff196f7b723289425dcb5b6c732dR228) is still needed. We also changed the `std::is_integral` to `rocprim::is_integral` to not include `(u)int128_t` for this specialization, this does not impact performance. The specialization enable_if was also slightly changed to make it clearer which path is chosen (does not make a difference in actual executed code). Also custom config and a iterator was added to the tests of merge_sort. In ROCm/rocm-libraries@f5614d5 test coverage was added for the device_scan_common.hpp file. In ROCm/rocm-libraries@1d2a40a test coverage was increased by actually using `const fixed_array` and including a `Level` type which goes into the base path of sample_to_bin_even struct. We also added an iterator as a type to the test. Also the new test wrapper was used. In ROCm/rocm-libraries@c518f42 test coverage was increased for the iterators, a lot of the operators where missing. Also some cleaning up of the tests was done including the wrapper when possible. There was also a bug found for the comperator in `arg_index_iterator` and `texture_cache_iterator`, which was also fixed. The `->` operator is not tested for `test_texture_cache`, `arg_index_iterator`, `transform_iterator` and `zip_iterator`. They currently do not seem to work, I am working on a possible fix but will be added in a later PR.
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.
In this commit we have added test coverage, made some bug fixes and added a wrapper function for a common pattern in the tests. This
test_kernel_wrapperfunction reduces the amount of code used.In
0318f42 we have added coverage of the
warp_scan_shuffleby forcing dpp off in cmake on thewarp_scantest.In
30d54c1 we have added the missing test cases for the thread_algos in the thread directory. (Except for thread_operators, these functions are mostly duplicates probably needs to be cleanly moved/deprecated). We also found a bug in
thread_reducewith the tests and fixed this.In
48780ff 37aa542 05a9e12 f5f1dbe 343e275 8007fe6 we have added unit tests for the files in the types directory.
In
40ca3e2 we have introduced the
test_kernel_wrapperand added the missingPartitionTwoWayFlagtest.In
7a4e7ba unit tests for the rocprim::tuple type are added.
In
29eac9a we have added extra test coverage by introducing a new type that will go into the untested path. We have also added this type to the benchmark to show this
specialization is still needed. We also changed the
std::is_integraltorocprim::is_integralto not include(u)int128_tfor this specialization, this does not impact performance. The specialization enable_if was also slightly changed to make it clearer which path is chosen (does not make a difference in actual executed code). Also custom config and a iterator was added to the tests of merge_sort.In
f5614d5 test coverage was added for the device_scan_common.hpp file.
In
1d2a40a test coverage was increased by actually using
const fixed_arrayand including aLeveltype which goes into the base path of sample_to_bin_even struct. We also added an iterator as a type to the test. Also the new test wrapper was used.In
c518f42 test coverage was increased for the iterators, a lot of the operators where missing. Also some cleaning up of the tests was done including the wrapper when possible. There was also a bug found for the comperator in
arg_index_iteratorandtexture_cache_iterator, which was also fixed. The->operator is not tested fortest_texture_cache,arg_index_iterator,transform_iteratorandzip_iterator. They currently do not seem to work, I am working on a possible fix but will be added in a later PR.Reopening this PR to trigger all CIs and labels. Original PR here: (#753)