Fix memcheck errors found in REDUCTION_TEST #13574
Merged
Conversation
This file contains 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
davidwendt
added
bug
|
The gtest was recoded slightly for clarity and to make this problem easier to debug. It looks larger only because of the reformatting. |
This was referenced Jun 14, 2023
davidwendt
changed the title
davidwendt
changed the title
ttnghia
approved these changes
Jun 14, 2023
nvdbaranec
approved these changes
Jun 14, 2023
3 tasks
|
/merge |
rapids-bot bot
pushed a commit
that referenced
this pull request
Jun 22, 2023
Fixes a memcheck error found in `STRINGS_TEST` where an `atomicOr` was used on a boolean device scalar. The workaround uses a `cub::WarpReduce` to compute the result in the warp-per-string kernel. Reference #13574 Authors: - David Wendt (https://github.com/davidwendt) Approvers: - Bradley Dice (https://github.com/bdice) - Nghia Truong (https://github.com/ttnghia) URL: #13578
rapids-bot bot
pushed a commit
that referenced
this pull request
Jul 10, 2023
Contributes to #13575 Depends on #13574, #13578 This PR cleans up custom atomic implementations in libcudf by using `cuda::atomic_ref` when possible. It removes atomic bitwise operations like `and`, `or` and `xor` since libcudac++ already provides proper replacements. Authors: - Yunsong Wang (https://github.com/PointKernel) Approvers: - Bradley Dice (https://github.com/bdice) - David Wendt (https://github.com/davidwendt) URL: #13583
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.
Description
The nightly tests found memcheck errors in
REDUCTION_TESTfor any/all aggregation types on dictionary columns.The error is caused by the internal
atomicOr()function which reads the target value as 32-bit even if the type is bool (8-bit).The fix works around this limitation by using
atomicOr()on a 32-bit device scalar and then converting that to a bool scalar on return.All other tests in
REDUCTION_TESTnow pass memcheck successfully.Closes #13572
Checklist