Unify return type of oneapi::dpl::__Internal::__ranges::__pattern_minmax_element
#2496
Conversation
This file contains hidden or 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
… struct __minmax_element_fn::operator()
…rror: incorrect return type from __pattern_minmax_element + __hetero_tag
… explicit return type with __pattern_minmax_element_impl() calls
…or_t in the namespace oneapi::dpl::__internal::__ranges
…turns pairs of iterators
This reverts commit 36cc203. # Conflicts: # include/oneapi/dpl/pstl/glue_algorithm_ranges_impl.h # include/oneapi/dpl/pstl/hetero/algorithm_ranges_impl_hetero.h
…oneapi::dpl::__ranges::__begin() / oneapi::dpl::__ranges::__end() methods
…ranges::__empty() method based on oneapi::dpl::__ranges::__begin() and oneapi::dpl::__ranges::__end() methods
…ranges::__size() method based on oneapi::dpl::__ranges::__begin() and oneapi::dpl::__ranges::__end() methods
…:__begin() in struct __subscription_impl_view_simple::operator[]
… oneapi::dpl::__ranges::__iterator_t in __pattern_min_element_impl + _BackendTag
… oneapi::dpl::__ranges::__iterator_t in __pattern_minmax_element_impl + _BackendTag
…internal::__value_t based on oneapi::dpl::__ranges::__iterator_t
…:dpl::__internal::__value_t based on oneapi::dpl::__ranges::__iterator_t" This reverts commit 56a1c26.
… / end() methods through ADL
… / end() methods for arrays
…__ranges::__begin() / oneapi::dpl::__ranges::__end() methods
…e _IteratorValueType through oneapi::dpl::__ranges::__iterator_t<_Range> in __pattern_min_element_impl + _BackendTag and __pattern_minmax_element_impl + _BackendTag methods
…terator_t as not used anymore
…ace before __begin() calls
…ienko/avoid_begin_end_calls_for_ranges
…f namespace before __begin() calls" This reverts commit 0b445fa.
…ern_minmax + _Tag : we shouldn't move the range __r into __pattern_minmax_element because after this call we dereference their iterator
…n_minmax_element() calls
…duce and apply in the code __pattern_minmax_element_impl_return_t for __pattern_minmax_element_impl return type
… renames in __pattern_minmax_element_impl + _BackendTag
…we shouldn't move range into because later using their iterators in returned value so they should still valid
SergeyKopienko
removed request for
akukanov,
danhoeflinger and
dmitriy-sobolev
…ror in minmax_element + _ExecutionPolicy
SergeyKopienko
changed the title
__pattern_minmax_elementoneapi::dpl::__Internal::__ranges::__pattern_minmax_element
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…ompile error in __pattern_minmax_element + __hetero_tag
SergeyKopienko
force-pushed
the
dev/skopienko/declare_types_for_pattern_minmax_element
branch
from
acde4d2 to
01a71e2
Compare
…eview comment: replace __rng.size() to oneapi::dpl::__ranges::__size(__rng) in __pattern_minmax_element + __hetero_tag - we are not under _ONEDPL_CPP20_RANGES_PRESENT macro in this code
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
…eview comment: replace __rng.begin() to oneapi::dpl::__ranges::__begin(__rng) in __pattern_minmax_element + __hetero_tag - we are not under _ONEDPL_CPP20_RANGES_PRESENT macro in this code
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
SergeyKopienko
requested review from
akukanov,
danhoeflinger and
dmitriy-sobolev
and removed request for
akukanov,
danhoeflinger and
dmitriy-sobolev
…n __pattern_minmax_element + __serial_tag
In file included from oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:16:
In file included from oneDPL/test/parallel_api/ranges/std_ranges_test.h:20:
In file included from oneDPL/include/oneapi/dpl/algorithm:38:
In file included from oneDPL/include/oneapi/dpl/pstl/glue_algorithm_ranges_impl.h:34:
oneDPL/include/oneapi/dpl/pstl/algorithm_ranges_impl.h:502:12: error: no viable conversion from returned value of type 'minmax_element_result<borrowed_iterator_t<vector<int, allocator<int>> &>>' (aka 'min_max_result<__normal_iterator<int *, std::vector<int, std::allocator<int>>>>') to function return type 'std::pair<oneapi::dpl::__ranges::__iterator_t<vector<int, allocator<int>> &>, oneapi::dpl::__ranges::__iterator_t<vector<int, allocator<int>> &>>' (aka 'pair<__normal_iterator<int *, std::vector<int, std::allocator<int>>>, __normal_iterator<int *, std::vector<int, std::allocator<int>>>>')
| return std::ranges::minmax_element(std::forward<_R>(__r), __comp, __proj);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
oneDPL/include/oneapi/dpl/pstl/algorithm_ranges_impl.h:514:9: note: in instantiation of function template specialization 'oneapi::dpl::__internal::__ranges::__pattern_minmax_element<const oneapi::dpl::execution::sequenced_policy &, std::vector<int> &, std::identity, std::ranges::less>' requested here
| __pattern_minmax_element(__tag, std::forward<_ExecutionPolicy>(__exec), __r, __comp, __proj);
| ^
oneDPL/include/oneapi/dpl/pstl/glue_algorithm_ranges_impl.h:703:73: note: in instantiation of function template specialization 'oneapi::dpl::__internal::__ranges::__pattern_minmax<oneapi::dpl::__internal::__serial_tag<std::integral_constant<bool, false>>, const oneapi::dpl::execution::sequenced_policy &, std::vector<int> &, std::identity, std::ranges::less>' requested here
| const auto& [__min, __max] = oneapi::dpl::__internal::__ranges::__pattern_minmax(__dispatch_tag,
| ^
oneDPL/test/parallel_api/ranges/std_ranges_test.h:373:20: note: in instantiation of function template specialization 'oneapi::dpl::ranges::__internal::__minmax_fn::operator()<const oneapi::dpl::execution::sequenced_policy &, std::vector<int> &, std::identity, std::ranges::less>' requested here
| auto res = algo(CLONE_TEST_POLICY(exec), r_in, args...);
| ^
oneDPL/test/parallel_api/ranges/std_ranges_test.h:261:9: note: in instantiation of function template specialization 'test_std_ranges::test<int, test_std_ranges::host_vector<int>>::process_data_in<const oneapi::dpl::execution::sequenced_policy &, oneapi::dpl::ranges::__internal::__minmax_fn, (lambda at oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:36:27), std::identity, std::ranges::less>' requested here
| process_data_in(max_n, CLONE_TEST_POLICY(exec), algo, checker, tr_in, args...);
| ^
oneDPL/test/parallel_api/ranges/std_ranges_test.h:251:9: note: in instantiation of function template specialization 'test_std_ranges::test<int, test_std_ranges::host_vector<int>>::operator()<const oneapi::dpl::execution::sequenced_policy &, oneapi::dpl::ranges::__internal::__minmax_fn, (lambda at oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:36:27), std::identity, std::identity, test_std_ranges::data_in, std::ranges::less>' requested here
| operator()(n_serial, oneapi::dpl::execution::seq, algo, checker, args...);
| ^
oneDPL/test/parallel_api/ranges/std_ranges_test.h:871:63: note: in instantiation of function template specialization 'test_std_ranges::test<int, test_std_ranges::host_vector<int>>::host_policies<oneapi::dpl::ranges::__internal::__minmax_fn, (lambda at oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:36:27), std::identity, std::identity, std::ranges::less>' requested here
| test<T, host_vector<T>, mode, DataGen1, DataGen2>{}.host_policies(n_serial, n_parallel, algo, checker, std::identity{}, std::identity{}, args...);
| ^
oneDPL/test/parallel_api/ranges/std_ranges_test.h:911:9: note: in instantiation of function template specialization 'test_std_ranges::test_range_algo<>::test_range_algo_impl_host<oneapi::dpl::ranges::__internal::__minmax_fn, (lambda at oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:36:27), std::ranges::less>' requested here
| test_range_algo_impl_host(algo, checker, args...);
| ^
oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:38:31: note: in instantiation of function template specialization 'test_std_ranges::test_range_algo<>::operator()<oneapi::dpl::ranges::__internal::__minmax_fn, (lambda at oneDPL/test/parallel_api/ranges/std_ranges_minmax.pass.cpp:36:27), std::ranges::less>' requested here
| test_range_algo<0>{big_sz}(dpl_ranges::minmax, minmax_checker, std::ranges::less{});
| ^
| __dispatch_tag, ::std::forward<_ExecutionPolicy>(__exec), views::all_read(::std::forward<_Range>(__rng)), | ||
| __comp); | ||
| auto __view = views::all_read(std::forward<_Range>(__rng)); | ||
| auto __v_begin = __view.begin(); |
There was a problem hiding this comment.
This version of minmax_element is a part of experimental API which can accept a SYCl buffer. In that case views::all_read returns a placeholder accessor.
SYCL spec tells:
"... a placeholder accessor is not yet bound to a command group. Before such an accessor can be used in a command, it must be bound by calling handler::require(). Passing a placeholder accessor as an argument to a command without first being bound to a command group with handler::require() will result in undefined behavior."
So, a result of accessor::begin() call is undefined behavior.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
This PR updates the return types of
oneapi::dpl::__Internal::__ranges::__pattern_minmax_elementrange-based functions to return pair of iterators instead pair of of indices, improving type consistency and API design. The changes include updating function signatures, modifying return statements to use iterator arithmetic, and adjusting variable declarations to use explicit type declarations.Justification: for compatibility with iterator-based version of
oneapi::dpl::__internal::__pattern_minmax_element:Note: make sense to review after landing #2493