-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SYCLomatic] Simplify segmented sort algorithms #1347
[SYCLomatic] Simplify segmented sort algorithms #1347
Conversation
Signed-off-by: Dan Hoeflinger <[email protected]>
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
@@ -2212,17 +2148,11 @@ segmented_sort_keys( | |||
dpct::internal::segmented_sort_keys_by_parallel_for_of_sorts( | |||
::std::forward<Policy>(policy), keys_in, keys_out, n, nsegments, | |||
begin_offsets, end_offsets, descending, begin_bit, end_bit); | |||
} else if (nsegments < 512) // for loop of parallel sorts when we have a small | |||
// number of total sorts to limit total overhead | |||
} else |
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.
This line need format
@@ -2272,19 +2202,12 @@ segmented_sort_pairs( | |||
::std::forward<Policy>(policy), keys_in, keys_out, values_in, | |||
values_out, n, nsegments, begin_offsets, end_offsets, descending, | |||
begin_bit, end_bit); | |||
} else if (nsegments < 512) // for loop of parallel sorts when we have a small | |||
// number of total sorts to limit total overhead | |||
} else |
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.
Same as above
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 with comments
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
Remove the two pair sorts algorithm options from segmented sort APIs. This shouldn't hurt much performance wise at runtime, as most cases will not reach this code, but it will significantly reduce the number of kernels required to JIT compile.
Must be preceded by merging of oneapi-src/SYCLomatic-test#483, to remove the direct testing of this internal API.