-
Notifications
You must be signed in to change notification settings - Fork 757
Use CUB version of merge sort #1511
Use CUB version of merge sort #1511
Conversation
run tests |
f00fabe
to
a212bb6
Compare
run tests |
a212bb6
to
6cc34dc
Compare
run tests |
DVS: 30345552 |
6cc34dc
to
83eaecc
Compare
run tests |
@senior-zero Let's try to get this in for 1.15 if DVS is passing and it's ready to merge, but this is super low priority if it's going to take actual effort. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
thrust/system/cuda/detail/sort.h
Outdated
|
||
for (int pass = 0; pass < num_passes; ++pass, ping = !ping) | ||
if (STABLE::value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These may need to be THRUST_IF_CONSTEXPR
if tests complain about the constexpr arg.
run tests |
13707fb
to
5a17e55
Compare
run tests |
5a17e55
to
f7e39a8
Compare
run tests |
DVS CL: 30537652 |
b16eae9
to
ce4c107
Compare
run tests |
Hoping DVS behaves this time 😅 DVS CL: 30549730 |
The DVS builds are all timing out with this update, and gpuCI seems to be taking about an hour longer than before. We should track down why and fix this before merging. Moving to 1.16 since this isn't urgent for 1.15. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to track down compile time increases between old and new merge sort implementations.
ce4c107
to
9dd2f22
Compare
run tests |
9dd2f22
to
6192d9a
Compare
run tests |
6192d9a
to
91d5e6c
Compare
This PR removes the Thrust version of merge sort and uses CUB one instead.