Skip to content

[onedpl][ranges] oneDPL range based API: M algorithms set implementation #2100

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 113 commits into from
May 5, 2025
Merged

[onedpl][ranges] oneDPL range based API: M algorithms set implementation #2100

merged 113 commits into from
May 5, 2025

Conversation

MikeDvorskiy
Copy link
Contributor

@MikeDvorskiy MikeDvorskiy commented Mar 5, 2025 •
edited by akukanov
Loading

[onedpl][ranges] oneDPL range based API: M algorithms set: fill, generate, move, replace, replace_if, remove, remove_if, mismatch, minmax_element, minmax, min, max, find_first_of, find_end, is_sorted_until

@MikeDvorskiy MikeDvorskiy marked this pull request as draft March 5, 2025 10:56
@MikeDvorskiy MikeDvorskiy force-pushed the dev/mdvorski/ranges_algo_M-origin_UXL branch 5 times, most recently from b2a1dc0 to b0ee21e Compare March 12, 2025 17:23
@MikeDvorskiy MikeDvorskiy force-pushed the dev/mdvorski/ranges_algo_M-origin_UXL branch 24 times, most recently from a1fa6d0 to 529c77c Compare March 20, 2025 16:19
@MikeDvorskiy MikeDvorskiy force-pushed the dev/mdvorski/ranges_algo_M-origin_UXL branch from 8f7099a to a774e1d Compare May 1, 2025 21:04
dmitriy-sobolev
dmitriy-sobolev reviewed May 1, 2025
View reviewed changes
dmitriy-sobolev
dmitriy-sobolev previously approved these changes May 2, 2025
View reviewed changes
Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM.

I've additionally tested this PR on different operating systems and compilers: no new issues noticed.

A non-critical point, which does not block the PR, and can be addressed separately: #2100 (comment)
RFC to be merged first: #2037.

@danhoeflinger
Copy link
Contributor

There are a good number of clang format suggestions I think are worth accepting.
I have no additional feedback for the code, and no objections to merging.

I haven't yet been able to check through the tests, but I trust @dmitriy-sobolev covering that. I should have some time to get to the tests later today but I don't expect to have issues.

danhoeflinger
danhoeflinger previously approved these changes May 2, 2025
View reviewed changes
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM as well. (but I would suggest considering some of the clang format suggestions.)

@dmitriy-sobolev dmitriy-sobolev dismissed stale reviews from danhoeflinger and themself via ce52a0b May 2, 2025 16:48
@dmitriy-sobolev dmitriy-sobolev force-pushed the dev/mdvorski/ranges_algo_M-origin_UXL branch from ce52a0b to 49b1120 Compare May 2, 2025 16:53
@dmitriy-sobolev dmitriy-sobolev force-pushed the dev/mdvorski/ranges_algo_M-origin_UXL branch from 49b1120 to 93ea564 Compare May 2, 2025 16:58
dmitriy-sobolev
dmitriy-sobolev approved these changes May 2, 2025
View reviewed changes
Copy link
Contributor

@dmitriy-sobolev dmitriy-sobolev left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I am reapproving after applying some clang-format suggestions.
I am a bit biased about them, but it is a tiny portion of the PR.

@MikeDvorskiy
Copy link
Contributor Author

MikeDvorskiy commented May 4, 2025 •
edited
Loading

some of the clang format suggestions.

Dan, could you please point which ones? Or do you mean the all suggestions?

@MikeDvorskiy
Copy link
Contributor Author

MikeDvorskiy commented May 4, 2025 •
edited
Loading

RFC to be merged first: #2037.

#2037 merged

@danhoeflinger
Copy link
Contributor

some of the clang format suggestions.

Dan, could you please point which ones? Or do you mean the all suggestions?

I didnt go through them very thoroughly after I saw a few suggestions which seemed to be obvious ones to accept. Also, my preference is that if something is on the edge, I also prefer to take the suggestion just because it will continue to show up in any subsequent changes.

I trust your judgement.

danhoeflinger
danhoeflinger approved these changes May 4, 2025
View reviewed changes
Copy link
Contributor

@danhoeflinger danhoeflinger left a comment

Choose a reason for hiding this comment

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

The clang format changes seem fine. Optional to take any other suggestions in my opinion.

@MikeDvorskiy MikeDvorskiy merged commit df22ab7 into main May 5, 2025
18 of 19 checks passed
@MikeDvorskiy MikeDvorskiy deleted the dev/mdvorski/ranges_algo_M-origin_UXL branch May 5, 2025 11:23
This was referenced May 19, 2025
Closed
Closed
@SergeyKopienko SergeyKopienko mentioned this pull request May 27, 2025
Open
timmiesmith pushed a commit that referenced this pull request Jun 9, 2025
@MikeDvorskiy @timmiesmith
[onedpl][ranges] oneDPL range based API: M algorithms set implementat…
c9e0503 
…ion (#2100)

* Based on RFC #2037
* [oneDPL][ranges][M] + fix: __unseq_backend::__simd_walk_1 -> __unseq_backend::__simd_walk_n
* [oneDPL][hetero] + changes walkX_vectors_or_scalars class: + 'mutable' is to relax the requirements for a user functor/lambda type operator() may be non-const
* [oneDPL][ranges][M] + ranges::generate removed; (there is still discussion in C++ standard committee)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mmichel11 mmichel11 mmichel11 left review comments

@dmitriy-sobolev dmitriy-sobolev dmitriy-sobolev approved these changes

@danhoeflinger danhoeflinger danhoeflinger approved these changes

@rarutyun rarutyun Awaiting requested review from rarutyun

@akukanov akukanov Awaiting requested review from akukanov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2022.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@MikeDvorskiy @danhoeflinger @akukanov @dmitriy-sobolev @mmichel11