-
Notifications
You must be signed in to change notification settings - Fork 120
RFC for zip_view implementation, for oneDPL C++20 #1931
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
Changes from 42 commits
09bd199
b56c860
761ecb7
7ef15f4
e792b5b
786d5c5
171e5fe
f9a2e45
4a701fc
cc3eac4
52acfac
526a603
518aee0
234dbdb
c20ef9f
aec760b
9ab570b
300e13c
f51297e
4cade93
7b882fb
2e9c98d
214dcde
1f7b132
720f1d2
416010b
28aed7e
1ce2407
20d82e8
87a32a8
c629dff
68d67f6
14118c6
e8fcc7c
13a814f
8e140b0
cc89449
cdf9db7
a86e51e
55f41cb
210fae7
5fef7a9
b28c2fe
ffaa838
d7fbd96
c23437d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # zip_view Support for the oneDPL Range APIs with C++20 | ||
|
|
||
| ## Introduction | ||
| `std::ranges::zip_view` is a powerful utility that enables developers to combine two or more ranges into a single view, | ||
| where each element is represented as a tuple containing corresponding elements from each input range. | ||
|
|
||
| ## Motivations | ||
| `std::ranges::zip_view` is a convenient way to combine multiple ranges into a single view, where each element of | ||
| the resulting range is a tuple containing one element from each of the input ranges. This can be particularly | ||
| useful for iterating over multiple collections in parallel. `std::ranges::zip_view` is added in C++23, | ||
| but many developers are still using C++20 standard. oneDPL introduces `oneapi::dpl::ranges::zip_view`, | ||
| with the same API and functionality as `std::ranges::zip_view`. | ||
|
|
||
| In case of C++23 `oneapi::dpl::ranges::zip_view` using also makes sense at least for the device policies, because | ||
| `std::ranges::zip_view` C++23 still is not device copyable. Any wrapper over `std::tuple` C++23 is not device copyable. (https://godbolt.org/z/brfvcMeM6) | ||
| There are another technical issues with `std::tuple` (see below for the details). | ||
|
|
||
| ### Key Requirements | ||
| `oneapi::dpl::ranges::zip_view` should be: | ||
| - compilable with C++20 version (minimum) | ||
|
dmitriy-sobolev marked this conversation as resolved.
Outdated
|
||
| - API-compliant with `std::ranges::zip_view` | ||
| - in case of a device usage: a device copyable view if the all "underlying" views are device copyable views. | ||
| - The implementation may be based on tuple-like type underhood, but it must provide a transitive device copyability. | ||
| - To satisfy trivially copyability to provide a transitive device copyability for the pipes created over `oneapi::dpl::ranges::zip_view`. | ||
|
|
||
| `oneapi::dpl::ranges::zip_view::iterator` should be: | ||
| - value-swappable (https://en.cppreference.com/w/cpp/named_req/ValueSwappable) | ||
|
akukanov marked this conversation as resolved.
|
||
| - indirectly writable (https://en.cppreference.com/w/cpp/iterator/indirectly_writable.html) | ||
| - able to be used with the non-range algorithms, including C++ and oneDPL parallel algorithms: | ||
| depending on the algorithm, stricter requirements may apply. For example, `std::sortable` concept must be satisfied in order to call `std::ranges::sort`. | ||
|
|
||
| ### Discrepancies with std::zip_view C++23 | ||
| - `oneapi::dpl::ranges::zip_view` may use a oneDPL tuple-like type instead of `std::tuple` | ||
| - `oneapi::dpl::ranges::zip_view::iterator::value_type` should not be defined as `std::tuple` (see a known technical issue below) | ||
|
|
||
| ### Other technical reasons not to use std::zip_view C++23 (and std::tuple) with oneDPL algorithms in the future: | ||
| - There is an issue with `std::ranges::sort(zip_view)` with clang 19.0 and older. (https://godbolt.org/z/jKvG9rY5M) | ||
| - There is an issue with `std::ranges::stable_sort(zip_view)` with gcc library | ||
| - Passing `std::zip_view::iterator` instances to the iterator-based algorithms works only for gcc 14.1 and newer, clang 19.1 and newer or | ||
| starting 17.01 with libc++ lib (https://godbolt.org/z/To6Mjr9M6) | ||
| - Considiration `std::tuple` as `oneapi::dpl::ranges::zip_view::iterator::value_type`. There are issues, at least, with `sortable`, `permutable` | ||
| and `indirectly_writable` concepts: const_cast<const std::iter_reference_t<Out>&&>(*o) = std::forward<T>(t) is not compiled till C++23. (https://godbolt.org/z/zT9qqnjWq) | ||
|
|
||
| ### Implementation proposal (C++20) | ||
| - `oneapi::dpl::ranges::zip_view` is as a C++ class representing a range adaptor (see C++ Range Library). | ||
| - The implementation derives from the C++ `std::ranges::view_interface`. | ||
| This class encapsulates a tuple-like type to keep a combination of two or more ranges. | ||
| - To ensure device copyability, `oneapi::dpl::__internal::tuple` is proposed as a tuple-like type for underlying elements. | ||
| - To provide a value-swappable requirement `oneapi::dpl::__internal::tuple` is proposed as a dereferenced value for | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When it comes to the specification, this will need to be described as an unspecified tuple which satisfies some set of requirements, unless we want to make the move to specify our internal tuple and make it public (which I doubt we want to do).
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the context of this, I was looking at the specification for our current
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are moving towards specifying the tuple-like type better, most likely as a type that satisfies certain concepts / requirements. That should be done in an new proposal though.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes.. I've mentioned it in the RFC document update. |
||
| `oneapi::dpl::ranges::zip_view::iterator` due to `std::tuple` not satisfying the value-swappable requirement in C++20. | ||
| - To provide a indirectly writable requirement `oneapi::dpl::__internal::tuple` is proposed as the public type for `oneapi::dpl::ranges::zip_view::iterator::value_type`. | ||
| - Usage of C++ concepts is desirable to write type requirements for types, methods and members of the class. | ||
| - C++20 is minimum supported version for the class. It allows using modern C++ features such as concepts and others. | ||
|
|
||
| ### Test coverage | ||
| - `oneapi::dpl::ranges::zip_view` is tested itself, base functionality (the API that is used for a range in the oneDPL algorithm implementations) | ||
| - the base functionality test coverage may be extended by the adapted LLVM `std::ranges::zip_view` (C++23) tests. | ||
| - should be tested with range based algorithms. | ||
| - should be tested with iterator based algorithms. | ||
Uh oh!
There was an error while loading. Please reload this page.