Implement Kokkos::sort with execution space #4490
Conversation
masterleinad
force-pushed
the
async_sort
branch
from
fc2547b
to
8849ff9
Compare
algorithms/src/Kokkos_Sort.hpp
Outdated
| if (!always_use_kokkos_sort) { | ||
| if (Impl::try_std_sort(view)) return; | ||
| } |
There was a problem hiding this comment.
This shortcut becomes interesting when passing an execution space. I need to think about it more.
There was a problem hiding this comment.
Yes, we can discuss this. In case, we want Kokkos::sort to behave as deep_copy, we should add a fence after std::sort so that the user can rely on the kernel being enqueued.
masterleinad
added
the
Upcoming Vote
masterleinad
force-pushed
the
async_sort
branch
from
27fc005
to
8e54fb3
Compare
masterleinad
removed
the
Upcoming Vote
masterleinad
force-pushed
the
async_sort
branch
2 times, most recently
from
b18ad85
to
8d3d7e8
Compare
masterleinad
force-pushed
the
async_sort
branch
from
8d3d7e8
to
3d2303b
Compare
|
|
|
Retest this please. |
|
Morning @masterleinad , @dalg24 and @PhilMiller -- Would you like to add this issue to the list of things we'll discuss tomorrow? If so, please just let me know. |
We did discuss this already last week. I don't think there is anything new to talk about. We just need more reviews here. |
| @@ -533,6 +533,7 @@ bool try_std_sort(ViewType view) { | |||
| possible = possible && (ViewType::Rank == 1); | |||
| possible = possible && (stride[0] == 1); | |||
| if (possible) { | |||
| exec.fence("Kokkos::sort: Fence before sorting on the host"); | |||
There was a problem hiding this comment.
Although I don't necessarily object to that change, it needs to be highlighted not just committed along with addressing another unrelated review comment.
There was a problem hiding this comment.
I split the commit.
masterleinad
force-pushed
the
async_sort
branch
from
07bfdc0
to
911237e
Compare
* Implement Kokkos::sort with execution space * Use Kokkos::sort overloads with execution space in unit tests * Use three-argument deep_copy in Kokkos_Sort * Use execution_space instead of DefaultExecutionSpace * Move fence() to versions not taking an execution space * Use given execution space for view allocations * Use default argument for create_permute_vector * Use class execution_space * Fence execution_space when using std::sort (cherry picked from commit 72418c2) Conflicts: algorithms/src/Kokkos_Sort.hpp
[3.5] Implement Kokkos::sort with execution space (#4490)
I saw multiple projects introducing extra fences for
Kokkos::sortbecause it couldn't take an execution space as an argument. This pull request adds the respective overloads.