Skip to content

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

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
09bd199
[oneDPL][rfc][zip_view] + draft
MikeDvorskiy Nov 4, 2024
b56c860
Update README.md
MikeDvorskiy Nov 27, 2024
761ecb7
Update README.md
MikeDvorskiy Nov 29, 2024
7ef15f4
Update README.md
MikeDvorskiy Nov 29, 2024
e792b5b
Update README.md
MikeDvorskiy Nov 29, 2024
786d5c5
Update README.md
MikeDvorskiy Nov 29, 2024
171e5fe
Update README.md
MikeDvorskiy Nov 29, 2024
f9a2e45
[oneDPL][rfc][ranges][zip_view] + Implementation design proposal
MikeDvorskiy Dec 2, 2024
4a701fc
+ spelling fix
MikeDvorskiy Dec 3, 2024
cc3eac4
+ spelling fix
MikeDvorskiy Dec 3, 2024
52acfac
+Implementation proposal
MikeDvorskiy Dec 4, 2024
526a603
Delete rfcs/experimental/zip_view/README.md
MikeDvorskiy Dec 4, 2024
518aee0
Update README.md
MikeDvorskiy Dec 4, 2024
234dbdb
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
c20ef9f
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
aec760b
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
9ab570b
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
300e13c
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
f51297e
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
4cade93
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
7b882fb
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
2e9c98d
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 17, 2024
214dcde
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Dec 18, 2024
1f7b132
Update README.md
MikeDvorskiy Dec 20, 2024
720f1d2
Update README.md
MikeDvorskiy Jan 15, 2025
416010b
Update README.md
MikeDvorskiy Jan 15, 2025
28aed7e
Update README.md
MikeDvorskiy Jan 15, 2025
1ce2407
Update README.md
MikeDvorskiy Jan 15, 2025
20d82e8
Update README.md
MikeDvorskiy Feb 24, 2025
87a32a8
Update README.md
MikeDvorskiy Feb 24, 2025
c629dff
Update README.md
MikeDvorskiy Feb 24, 2025
68d67f6
Update README.md
MikeDvorskiy Mar 27, 2025
14118c6
Update README.md
MikeDvorskiy Mar 27, 2025
e8fcc7c
Update README.md
MikeDvorskiy Mar 27, 2025
13a814f
Update README.md
MikeDvorskiy Mar 28, 2025
8e140b0
Update rfcs/proposed/zip_view/README.md
MikeDvorskiy Apr 10, 2025
cc89449
Update README.md
MikeDvorskiy Apr 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 57 additions & 0 deletions rfcs/proposed/zip_view/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# 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` was introduced in C++23,
but many developers are still using C++20 standard. So, 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).
So, oneDPL tuple-like concept should be proposed into the oneDPL spec.

### Key Requirements
`oneapi::dpl::ranges::zip_view` should be:
- compilable with C++20 version (minimum)
- 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.
- To provide a transitive device copyability oneDPL tuple-like type underhood is proposed - `oneapi::dpl::__internal::tuple`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think which tuple to use is an implementation detail. What we really want is that our zip_view is trivially copyable if all the underlying views are - this is the way to have transitive device copyability for view pipelines.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think "to have transitive device copyability for view pipelines" - very strong argument to want that our zip_view is trivially copyable.
BTW, the proposed implementation has such property. The relevant test also was added.

- 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should explicitly add indirectly_writable as well.

- able to be used with the non-range algorithms, including C++ and oneDPL parallel algorithms

### Discrepancies with std::zip_view C++23
- `oneapi::dpl::ranges::zip_view` is based on oneDPL tuple-like type oneapi::dpl::__internal::tuple instead of std::tuple.
- `oneapi::dpl::ranges::zip_view::iterator::value_type` is oneDPL tuple-like type oneapi::dpl::__internal::tuple instead of std::tuple.
Comment on lines +32 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we mean that the API of zip_view and its iterator may use a oneDPL tuple-like type instead of std::tuple.
The details like oneapi::dpl::__internal::tuple are already mentioned in the implementation section 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` intances 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 designed as a C++ class which represents a range adaptor (see C++ Range Library).
- The implementation is based on 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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).

Copy link
Contributor

Choose a reason for hiding this comment

The 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 zip_iterator, and noticed a couple of discrepancies as compared to our implementation with regard to std::tuple vs our unspecified internal tuple type. I believe that our implementation is intentional and the "correct" thing here and the spec should be changed, because of issues with device copyability when composing types from zip_iterators. std::tuple is not trivially copyable but our internal tuple is.

uxlfoundation/oneAPI-spec#605

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.
- 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` tests.
- should be tested with at least one oneDPL range based algorithm.
- should be tested with at least one oneDPL iterator based algorithm.
Loading