Removal `ExecutionPolicy` from submitters and etc. stuff #2161

SergeyKopienko · 2025-04-03T08:12:19Z

Attention!

Now this PR still include all changes from

Replacement of internal lambda in oneDPL code by functional objects #2153

UPD: #2153 has been merged into main branch and this PR now contains actual state of main branch.

Before future work with this PR required to merge into main branch the next PRs (otherwise we will have a lot of differences in this PR):

The goal of this PR - pass into submitters sycl::queue& __q instead of _ExecutionPolicy&& __exec - so we remove one template parameter and decrease possible instantiations of template submitter classes / functions.

This is the main and final step to use ExecutionPolicy in oneDPL algorithm calls independently from their category values (when unnamed lambda is switched off or Kernel name is specified in the users code directly).

Note about `parallel_for`- and `parallel_for`-based submitters

Now we still need to have ExecutionPolicy in parallel_for- and in parallel_for-based submitters.
I going to fix that in the separate PR #2192

Note about clang-format check for this PR

I propose do not apply the rest of clang-format changes into this PR: please take a loot to them at https://github.com/uxlfoundation/oneDPL/actions/runs/14240611126/job/39909462269?pr=2161

Example (1) of incorrect clang-format:

         // 2. Apply work group carry outs, calculate output indices, and load results into correct indices.
-        __q.submit([&](sycl::handler& __cgh) {
+        __q
+            .submit([&](sycl::handler& __cgh) {
                 oneapi::dpl::__ranges::__require_access(__cgh, __keys, __out_values);

Example (2) of incorrect clang-format:

 template <typename _KernelName, ::std::uint32_t __radix_bits, bool __is_ascending, typename _ValRange,
           typename _CountBuf, typename _Proj
 #if _ONEDPL_COMPILE_KERNEL
-          , typename _Kernel
+          ,
+          typename _Kernel
 #endif
           >

Example (3) of incorrect clang-format:

 sycl::event
__radix_sort_count_submit(sycl::queue& __q, ::std::size_t __segments, ::std::siz
                           ::std::uint32_t __radix_offset, _ValRange&& __val_rng, _CountBuf& __count_buf,
                           sycl::event __dependency_event, _Proj __proj
 #if _ONEDPL_COMPILE_KERNEL
-                          , _Kernel& __kernel
+                          ,
+                          _Kernel& __kernel
 #endif
 )

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__device_info

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_work_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__slm_adjusted_work_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_sub_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_compute_units

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__supports_sub_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_work_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_sub_group_size

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_compiler::__compile

…remove _ExecutionPolicy template param from oneapi::dpl::__par_backend_hetero::__result_and_scratch_storage

…remove _ExecutionPolicy template param from oneapi::dpl::__par_backend_hetero::__internal::__print_device_debug_info

…__max_work_group_size

…__slm_adjusted_work_group_size

…__max_sub_group_size

…__max_compute_units

…__supports_sub_group_size

…__kernel_work_group_size

…__kernel_sub_group_size

…__kernel_compiler::__compile

…d_hetero::__internal::__print_device_debug_info

…d_hetero::__result_and_scratch_storage

…ch_storage

…remove template alias __result_and_scratch_storage and rename __result_and_scratch_storage_impl to __result_and_scratch_storage

…nd_impl_fn_fo to __lower_bound_impl_fn

mmichel11 · 2025-04-16T20:53:52Z

I looked through the PR and think it's near ready to merge.

There are a few clang-format suggestions from the ::std -> std changes I think. I suggest addressing those and am then ready to approve. The other clang-format suggestions I agree with ignoring.

… change

SergeyKopienko · 2025-04-17T07:21:26Z

There are a few clang-format suggestions from the ::std -> std changes I think. I suggest addressing those and am then ready to approve. The other clang-format suggestions I agree with ignoring.

I propose to stop this changes here. I don't think that it is real reason to not land this PR into main branch.

…_submitters # Conflicts: # include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_reduce.h

mmichel11

LGTM

adamfidel

Thanks for applying most of the suggestions. I agree where it doesn't make sense to change things that I suggested.

LGTM.

…tuff (#2161)" This reverts commit 14c97e0. # Conflicts: # include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_fpga.h # include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_histogram.h # include/oneapi/dpl/pstl/hetero/histogram_impl_hetero.h

rarutyun · 2025-04-17T17:54:14Z

Sorry guys, but this PR violates our agreements about oneDPL architecture. We cannot get execution policy on the patterns level (aglorithm_impl_hetero.h) because this is not a backend specific code. Yes, it's hetero specific but not the backend one. During our technical meetings Alexey and I explained that ExecutionPolicy is a backend property. So, going from policy to a sycl::queue should happen after we entered the backend. As the way to check yourself: one should call backend specific API on policy when the dispatch happened by backend tag (e.g., device_backend_tag) and should NOT call backend specific API when dispatch is done by hetero_tag.

@adamfidel, @mmichel11, I suggest you to be more careful in the future and ask if you are not sure what decisions were made.

@danhoeflinger, @MikeDvorskiy, I expect you to take a lead and scrutinize the PRs with architectural changes (like this one) if neither Alexey, not myself are in reviewers list.

It needs to be fixed before oneDPL release. It's either rolling back this change and making another one after rollback. Or it's just fix on top of that. I don't know what is the safest way because I didn't participate in the review, so I'll let you guys to decide.

SergeyKopienko · 2025-04-17T17:59:40Z

Sorry guys, but this PR violates our agreements about oneDPL architecture. We cannot get execution policy on the patterns level (aglorithm_impl_hetero.h) because this is not a backend specific code. Yes, it's hetero specific but not the backend one. During our technical meetings Alexey and I explained that ExecutionPolicy is a backend property. So, going from policy to a sycl::queue should happen after we entered the backend. As the way to check yourself: one should call backend specific API on policy when the dispatch happened by backend tag (e.g., device_backend_tag) and should NOT call backend specific API when dispatch is done by hetero_tag.

@adamfidel, @mmichel11, I suggest you to be more careful in the future and ask if you are not sure what decisions were made.

@danhoeflinger, @MikeDvorskiy, I expect you to take a lead and scrutinize the PRs with architectural changes (like this one) if neither Alexey, not myself are in reviewers list.

It needs to be fixed before oneDPL release. It's either rolling back this change and making another one after rollback. Or it's just fix on top of that. I don't know what is the safest way because I didn't participate in the review, so I'll let you guys to decide.

I propose to rollback this PR right now.
PR prepared: #2197

#2161 (#2197)

…etc. stuff #2161 (#2197)" This reverts commit 87ad0a6.

SergeyKopienko · 2025-04-17T21:39:11Z

Sorry guys, but this PR violates our agreements about oneDPL architecture. We cannot get execution policy on the patterns level (aglorithm_impl_hetero.h) because this is not a backend specific code. Yes, it's hetero specific but not the backend one. During our technical meetings Alexey and I explained that ExecutionPolicy is a backend property. So, going from policy to a sycl::queue should happen after we entered the backend. As the way to check yourself: one should call backend specific API on policy when the dispatch happened by backend tag (e.g., device_backend_tag) and should NOT call backend specific API when dispatch is done by hetero_tag.

@adamfidel, @mmichel11, I suggest you to be more careful in the future and ask if you are not sure what decisions were made.

@danhoeflinger, @MikeDvorskiy, I expect you to take a lead and scrutinize the PRs with architectural changes (like this one) if neither Alexey, not myself are in reviewers list.

It needs to be fixed before oneDPL release. It's either rolling back this change and making another one after rollback. Or it's just fix on top of that. I don't know what is the safest way because I didn't participate in the review, so I'll let you guys to decide.

PR with new implementation prepared: #2198

)

#2161 (#2197)

SergeyKopienko added 30 commits March 31, 2025 10:30

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

3e83786

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__device_info

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

62b4da7

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_work_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

71e6661

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__slm_adjusted_work_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

33cfdf4

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_sub_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

ec9461d

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__max_compute_units

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

da0c816

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__supports_sub_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

ca00220

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_work_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

77376ed

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_sub_group_size

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

e54ba10

…remove _ExecutionPolicy template param from oneapi::dpl::__internal::__kernel_compiler::__compile

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

6b30a46

…remove _ExecutionPolicy template param from oneapi::dpl::__par_backend_hetero::__result_and_scratch_storage

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

b2383e6

…remove _ExecutionPolicy template param from oneapi::dpl::__par_backend_hetero::__internal::__print_device_debug_info

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

0aadb2e

…__max_work_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

92202fa

…__slm_adjusted_work_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

805ff2b

…__max_sub_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

e98a152

…__max_compute_units

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

f411036

…__supports_sub_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

3fe9c8c

…__kernel_work_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

bc6b8e4

…__kernel_sub_group_size

Remove _ExecutionPolicy template param from oneapi::dpl::__internal::…

31a5a8f

…__kernel_compiler::__compile

Remove _ExecutionPolicy template param from oneapi::dpl::__par_backen…

c7bb1f1

…d_hetero::__internal::__print_device_debug_info

Remove _ExecutionPolicy template param from oneapi::dpl::__par_backen…

4525d4c

…d_hetero::__result_and_scratch_storage

Try to avoid declare sycl::queue __q as mutable in __result_and_scrat…

acd32b6

…ch_storage

Small refactoring of __result_and_scratch_storage

bcb1cc8

Apply GitHUB clang format

fe32b85

include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h - …

a57adf9

…remove template alias __result_and_scratch_storage and rename __result_and_scratch_storage_impl to __result_and_scratch_storage

Mark lambdas to move in ./include

4d9cf06

Move lambda definitions outside of template code in ./include

8b142a8

Remove KSATODO

5813665

Apply GitHUB clang format

9cfca2e

include/oneapi/dpl/internal/binary_search_impl.h - rename __lower_bou…

b77503c

…nd_impl_fn_fo to __lower_bound_impl_fn

SergeyKopienko requested a review from mmichel11 April 16, 2025 14:19

SergeyKopienko added 4 commits April 16, 2025 17:34

Replace ::std to std in changed lines of code

5f14fdf

Replace ::std to std in changed lines of code

b439db7

Replace ::std to std in changed lines of code

940f89f

Replace ::std to std in changed lines of code

061a8db

include/oneapi/dpl/pstl/hetero/algorithm_impl_hetero.h - remove extra…

882b523

… change

Apply GitHUB clang format

85a529e

SergeyKopienko mentioned this pull request Apr 17, 2025

Remove ExecutionPolicy from parallel_for- and parallel_for-based submitters #2192

Closed

Merge branch 'main' into dev/skopienko/3_remove_execution_policy_from…

7c66e48

…_submitters # Conflicts: # include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_reduce.h

SergeyKopienko linked an issue Apr 17, 2025 that may be closed by this pull request

Some __kernel_name_generator usages doesn't depends from _ExecutionPolicy type #2083

Closed

mmichel11 approved these changes Apr 17, 2025

View reviewed changes

adamfidel approved these changes Apr 17, 2025

View reviewed changes

SergeyKopienko merged commit 14c97e0 into main Apr 17, 2025
18 of 19 checks passed

SergeyKopienko deleted the dev/skopienko/remove_execution_policy_from_submitters branch April 17, 2025 15:04

mmichel11 mentioned this pull request Apr 17, 2025

Resolve C++20 extension warning in histogram_impl_hetero.h #2196

Closed

SergeyKopienko mentioned this pull request Apr 17, 2025

Revert: Removal ExecutionPolicy from submitters and etc. stuff #2161 #2197

Merged

SergeyKopienko restored the dev/skopienko/remove_execution_policy_from_submitters branch April 17, 2025 18:03

SergeyKopienko added a commit that referenced this pull request Apr 17, 2025

[oneDPL] Revert: Removal ExecutionPolicy from submitters and etc. stuff

87ad0a6

#2161 (#2197)

SergeyKopienko added a commit that referenced this pull request Apr 17, 2025

Revert "[oneDPL] Revert: Removal ExecutionPolicy from submitters and …

495c227

…etc. stuff #2161 (#2197)" This reverts commit 87ad0a6.

SergeyKopienko mentioned this pull request Apr 17, 2025

Removal ExecutionPolicy from submitters and etc. stuff (attemp #2) #2198

Merged

danhoeflinger mentioned this pull request Apr 23, 2025

Set Algorithms Performance Improvements #2147

Merged

SergeyKopienko deleted the dev/skopienko/remove_execution_policy_from_submitters branch April 29, 2025 20:21

timmiesmith pushed a commit that referenced this pull request Jun 9, 2025

[oneDPL] Removal ExecutionPolicy from submitters and etc. stuff (#2161

0938db7

)

timmiesmith pushed a commit that referenced this pull request Jun 9, 2025

[oneDPL] Revert: Removal ExecutionPolicy from submitters and etc. stuff

14ff430

#2161 (#2197)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Removal `ExecutionPolicy` from submitters and etc. stuff #2161

Removal `ExecutionPolicy` from submitters and etc. stuff #2161

Uh oh!

SergeyKopienko commented Apr 3, 2025 •

edited

Loading

Uh oh!

mmichel11 commented Apr 16, 2025 •

edited

Loading

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

mmichel11 left a comment

Uh oh!

adamfidel left a comment

Uh oh!

Uh oh!

rarutyun commented Apr 17, 2025

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

Uh oh!

Removal ExecutionPolicy from submitters and etc. stuff #2161

Removal ExecutionPolicy from submitters and etc. stuff #2161

Uh oh!

Conversation

SergeyKopienko commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Attention!

Note about parallel_for- and parallel_for-based submitters

Note about clang-format check for this PR

Uh oh!

mmichel11 commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

mmichel11 left a comment

Choose a reason for hiding this comment

Uh oh!

adamfidel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rarutyun commented Apr 17, 2025

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

SergeyKopienko commented Apr 17, 2025

Uh oh!

Uh oh!

Removal `ExecutionPolicy` from submitters and etc. stuff #2161

Removal `ExecutionPolicy` from submitters and etc. stuff #2161

SergeyKopienko commented Apr 3, 2025 •

edited

Loading

Note about `parallel_for`- and `parallel_for`-based submitters

mmichel11 commented Apr 16, 2025 •

edited

Loading