-
Notifications
You must be signed in to change notification settings - Fork 113
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
[onedpl][ranges][productization] Range API productization, MVP plan, Part 2 #1471
Conversation
b35c967
to
c081adf
Compare
dac7c2a
to
5d60dee
Compare
18e86e4
to
1502c28
Compare
1ffbab0
to
87b43a3
Compare
5cfde5d
to
946e7aa
Compare
946e7aa
to
a403829
Compare
c199049
to
0f656ba
Compare
a403829
to
f79d560
Compare
0f656ba
to
fbe0434
Compare
18a81d5
to
8ea5db8
Compare
72e28c8
to
850b408
Compare
8ea5db8
to
4e8aa56
Compare
850b408
to
212af8f
Compare
4e8aa56
to
b968c95
Compare
212af8f
to
153c036
Compare
b968c95
to
1b60415
Compare
106fe1b
to
4baf3c6
Compare
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
…std_ranges_support_mvp2
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
59fa835
to
c942acc
Compare
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
…std_ranges_support_mvp2
I've found that the tests for ranges fail for clang++14 when using libstdc++. This issue llvm/llvm-project#52696 looks the same. Basing on the reproducer from the issue, it affects clang++15 and older. I was not about to find a workaround after a quick look. With libc++ I got this issue: /oneDPL/test/parallel_api/ranges/std_ranges_transform.pass.cpp:77:28: error: use of undeclared identifier '__cpp_lib_ranges'
return TestUtils::done(_ENABLE_STD_RANGES_TESTING);
^
/oneDPL/test/support/test_config.h:120:36: note: expanded from macro '_ENABLE_STD_RANGES_TESTING'
#define _ENABLE_STD_RANGES_TESTING _ONEDPL_CPP20_RANGES_PRESENT
^
/oneDPL/include/oneapi/dpl/internal/../pstl/onedpl_config.h:307:43: note: expanded from macro '_ONEDPL_CPP20_RANGES_PRESENT'
# define _ONEDPL_CPP20_RANGES_PRESENT (__cpp_lib_ranges >= 201911L)
This definitely should be fixable. Upd. the first issue is related to the fact that clang++15 and older do not support range adapters. Reproducer: https://godbolt.org/z/hqd9qrfch. I've turned off the ranges with these compilers in MVP1. Upd. the second issue is fixed in MVP1 |
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
…std_ranges_support_mvp2
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
…std_ranges_support_mvp2
@@ -652,6 +757,19 @@ __pattern_sort(__hetero_tag<_BackendTag>, _ExecutionPolicy&& __exec, _Range&& __ | |||
.__deferrable_wait(); | |||
} | |||
|
|||
#if _ONEDPL_CPP20_RANGES_PRESENT | |||
template <typename _BackendTag, typename _ExecutionPolicy, typename _R, typename _Proj, typename _Comp> |
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.
Minor suggestion: align template parameter order with the function argument order.
template <typename _BackendTag, typename _ExecutionPolicy, typename _R, typename _Proj, typename _Comp> | |
template <typename _BackendTag, typename _ExecutionPolicy, typename _R, typename _Comp, typename _Proj> |
oneapi::dpl::__internal::__pattern_sort(__tag, std::forward<_ExecutionPolicy>(__exec), std::ranges::begin(__r), | ||
std::ranges::begin(__r) + std::ranges::size(__r), __comp_2); |
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.
We need separate patterns for std::ranges::sort
due to std::indirectly_swappable
requirement, which implies the use of std::ranges::iter_swap
, which can be customized externally.
oneapi::dpl::__internal::__pattern_sort
will use std::sort
as a brick, which uses std::sort
as a brick instead of std::range::sort
.
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.
I've added such patterns in this patch:
add_sort_ranges_patterns.patch
@rarutyun, lets discuss whether we want to include this patch in this PR or not.
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.
I've created a branch containing these changes: #1815
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com>
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.
Assuming the CI (except clang-format) is green I am totally for merging this PR. There might be some small improvements/fixes before code freeze but we definitely don't have enough time to make any significant improvements
…Part 2 (#1471) Signed-off-by: Dmitriy Sobolev <dmitriy.sobolev@intel.com> Co-authored-by: Ruslan Arutyunyan <ruslan.arutyunyan@intel.com> Co-authored-by: Dmitriy Sobolev <Dmitriy.Sobolev@intel.com> Co-authored-by: Julian Miller <julian.miller@intel.com>
[onedpl][ranges][productization] Range API productization, MVP plan, Part 2: count, count_if, equal, is_sorted, sort, stable_sort, min_element, max_element, copy, copy_if, merge.