[3.7.01] fix incorrect offset in cuda parallel scan for < 4 byte types #5607
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
…#5555) * kokkos#5545: tests: add reproducer test for kokkos#5545 * kokkos#5545: cuda: use word_size_type for ParallelScan * kokkos#5545: tests: add reproducer for offset issue with ParallelScanWithTotal * kokkos#5545: cuda: add word_size_type for ParallelScanWithTotal * fix formatting * cuda: fix comments for word_size_type * tests: fix mismatched types in Scan test assert * Fix HIP parallel_scan when using types < 4 bytes * Fix indentation * Replace class with typename * tests: re-use old scan test for small types; add extra parameter to avoid overflow * tests: remove fence and change ImbalanceSz to value_type * Fix format * Re-introduce size_type member type in ParallelScan[WithTotal] CUDA specializations Applied review suggestion to reduce the diff * Nitpick in HIP too prefere size_type alias to HIP::size_type in definition of word_size_type * Apply Dongs suggestion and revert one more change I missed Co-authored-by: Bruno Turcksin <bruno.turcksin@gmail.com> Co-authored-by: Damien L-G <dalg24+github@gmail.com> Co-authored-by: Damien L-G <dalg24@gmail.com>
nmm0
requested changes
Oct 31, 2022
There was a problem hiding this comment.
On line Kokkos_Cuda_ParallelRange.hpp:1060 (I can't comment directly on the diff since the line wasn't changed) I think we need:
m_scratch_space + (grid_x - 1) * size / sizeof(word_size_type),
size);size there is the value size so without this we may run into the same problem (offset of 0).
nmm0
previously requested changes
Nov 1, 2022
masterleinad
approved these changes
Nov 2, 2022
Rombur
approved these changes
Nov 2, 2022
dalg24
deleted the
rc371-cherrypick5555-fixincorrectoffsetparallelscansmalltypescudaandhip
branch
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.
Cherry-picking #5555 into RC 3.7.01
WARNING I had to do non-trivial conflict resolution in
Kokkos_Cuda_Parallel_Range.hpp,Kokkos_HIP_Parallel_Range.hpp, andKokkos_HIP_ReduceScan.hppbecause of #5146 and #5383Please review carefully.