Simplify `new_kernel_name` implementation in tests #2128

SergeyKopienko · 2025-03-13T12:35:08Z

This PR centralizes and simplifies the new_kernel_name machinery for tests by moving common templates into a single header and removing redundant definitions elsewhere.

Introduce uniq_kernel_index, unique_kernel_name, and new_kernel_name in utils_sycl_defs.h
Remove duplicated template definitions from utils_invoke.h, test_dynamic_selection_utils.h, and test_dynamic_load_utils.h
Include utils_sycl_defs.h where those utilities were previously declared inline

danhoeflinger

Can we take this opportunity to consolidate this to a single instance of this implementation?

danhoeflinger · 2025-03-13T15:56:23Z

Is there a reason there are 2 nested copies of TestUtils::unique_kernel_name here?
Are they redundant? If we change the number from 0 to 1, in your code, do both copies of the TestUtils::unique_kernel_name change their number? If so, there is some other issue we can fix I think.

SergeyKopienko · 2025-03-13T16:01:15Z

Is there a reason there are 2 nested copies of TestUtils::unique_kernel_name here? Are they redundant? If we change the number from 0 to 1, in your code, do both copies of the TestUtils::unique_kernel_name change their number? If so, there is some other issue we can fix I think.

In any case now new_kernel_name is implemented in this PR exactly as

template <typename Policy>
using __policy_kernel_name = typename ::std::decay_t<Policy>::kernel_name;

danhoeflinger · 2025-03-13T16:05:18Z

In any case now new_kernel_name is implemented in this PR exactly as
template <typename Policy>
using __policy_kernel_name = typename ::std::decay_t<Policy>::kernel_name;

Yes, but in your "after" result, we still have 2 nested TestUtils::unique_kernel_name. Perhaps that is because of nesting in the test function you are using to generate the "before" and "after", and is required and intentional.

SergeyKopienko · 2025-03-13T16:18:58Z

In any case now new_kernel_name is implemented in this PR exactly as
template <typename Policy>
using __policy_kernel_name = typename ::std::decay_t<Policy>::kernel_name;
Yes, but in your "after" result, we still have 2 nested TestUtils::unique_kernel_name. Perhaps that is because of nesting in the test function you are using to generate the "before" and "after", and is required and intentional.

As example I have used the policy creation at

oneDPL/test/parallel_api/algorithm/alg.sorting/alg.binary.search/binary.search/binary_search.pass.cpp

Line 74 in f5bd154

auto new_policy = make_new_policy<new_kernel_name<Policy, 0>>(exec);

:

auto new_policy = make_new_policy<new_kernel_name<Policy, 0>>(exec);

at this point the type of exec is oneapi::dpl::execution::__dpl::device_policy<TestUtils::unique_kernel_name<test_binary_search<unsigned long long>,0>> &
and the type of new_policy is
oneapi::dpl::execution::__dpl::device_policy<TestUtils::unique_kernel_name<TestUtils::unique_kernel_name<test_binary_search<unsigned long long>,0>,0>>

So I think it's the question for test environment staff, not for new_kernel_name :

this type name starts from

template <::std::size_t CallNumber = 0>
struct invoke_on_all_hetero_policies::operator()(Op op, Args&&... rest)

… in tests

…e implementation in tests

…lementation in tests

…lementation

…_impl

…me_impl

…_impl

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/support/utils_sycl_defs.h

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/support/utils_sycl_defs.h

…TestUtils::TestPolicyName' named in nested name specifier Building CXX object test/CMakeFiles/header_inclusion_order_async_0.pass.dir/general/header_inclusion_order_async_0.pass.cpp.o In file included from /test/general/header_inclusion_order_async_0.pass.cpp:19: In file included from /test/support/utils.h:47: In file included from /test/support/utils_test_base.h:25: In file included from /test/support/utils_invoke.h:25: /test/support/utils_sycl_defs.h:53:1: error: incomplete type 'TestUtils::TestPolicyName' named in nested name specifier 53 | using new_kernel_name = unique_kernel_name<typename std::decay_t<Policy>::kernel_name, idx>; | ^~~~~ /test/support/utils_invoke.h:132:39: note: in instantiation of template type alias 'new_kernel_name' requested here 132 | using _NewKernelName = TestUtils::new_kernel_name<PolicyName, call_id>; | ^ /test/general/header_inclusion_order_async_0.pass.cpp:26:30: note: in instantiation of function template specialization 'TestUtils::get_dpcpp_test_policy<0, TestUtils::TestPolicyName>' requested here 26 | auto policy = TestUtils::get_dpcpp_test_policy(); | ^ /test/support/utils_invoke.h:126:8: note: forward declaration of 'TestUtils::TestPolicyName' 126 | struct TestPolicyName; | ^ 1 error generated.

In file included from test/xpu_api/language.support/support.limits/limits/quiet_NaN.pass.cpp:20: In file included from test/support/test_complex.h:23: In file included from test/support/utils.h:47: In file included from test/support/utils_test_base.h:25: In file included from test/support/utils_invoke.h:25: test/support/utils_sycl_defs.h:24:14: fatal error: 'CL/sycl.hpp' file not found 24 | # include <CL/sycl.hpp> | ^~~~~~~~~~~~~

…index function should be marked inline to avoid potential ODR (One Definition Rule) violations when this header is included in multiple translation units

…mplate parameter name idx is inconsistent with CallNumber used in other templates in this file

Copilot

Pull Request Overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

test/support/utils_sycl_defs.h

test/support/utils_invoke.h

Co-authored-by: Copilot <[email protected]>

danhoeflinger

As long as tests work after this, the changes LGTM.
(may require more than the per-commit CI to cover this well either locally or in the CI)

SergeyKopienko added enhancement test Test only Change labels Mar 13, 2025

SergeyKopienko added this to the 2022.9.0 milestone Mar 13, 2025

SergeyKopienko requested review from MikeDvorskiy, dmitriy-sobolev, egfefey and rarutyun March 13, 2025 12:35

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch from eae40da to fc30465 Compare March 13, 2025 15:08

SergeyKopienko requested a review from danhoeflinger March 13, 2025 15:50

danhoeflinger reviewed Mar 13, 2025

View reviewed changes

SergeyKopienko requested a review from danhoeflinger March 13, 2025 16:01

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch 2 times, most recently from 9ea0453 to d3cff9c Compare March 26, 2025 08:47

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch 2 times, most recently from d19308f to 592fc8c Compare March 27, 2025 19:00

SergeyKopienko modified the milestones: 2022.9.0, 2022.10.0 Apr 3, 2025

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch 3 times, most recently from eae2814 to db08779 Compare April 4, 2025 13:06

SergeyKopienko added 4 commits April 8, 2025 16:55

test/support/utils_invoke.h - simplify new_kernel_name implementation…

f648190

… in tests

test/support/test_dynamic_selection_utils.h - simplify new_kernel_nam…

dbeb7af

…e implementation in tests

test/support/test_dynamic_load_utils.h - simplify new_kernel_name imp…

b11791e

…lementation in tests

Fix review comment: consolidate this to a single instance of this imp…

b2f1493

…lementation

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch from db08779 to b2f1493 Compare April 8, 2025 14:55

Merge branch 'main' into dev/skopienko/simplify_tests_new_kernel_name…

bdc044e

…_impl

SergeyKopienko added 4 commits September 26, 2025 17:09

Merge branch 'main' into dev/skopienko/2_simplify_tests_new_kernel_na…

4b77e7f

…me_impl

Merge branch 'main' into dev/skopienko/2_simplify_tests_new_kernel_na…

0b2353c

…me_impl

test/support/test_dynamic_load_utils.h - add missed include

24496bf

Merge branch 'main' into dev/skopienko/2_simplify_tests_new_kernel_na…

a9a1331

…me_impl

SergeyKopienko modified the milestones: 2022.11.0, 2022.12.0 Oct 27, 2025

SergeyKopienko added 3 commits October 31, 2025 17:28

Merge branch 'main' into dev/skopienko/2_simplify_tests_new_kernel_na…

ea9dbdb

…me_impl

Merge branch 'main' into dev/skopienko/2_simplify_tests_new_kernel_na…

9777bf8

…me_impl

Merge branch 'main' into dev/skopienko/simplify_tests_new_kernel_name…

323f0f7

…_impl

SergeyKopienko removed the enhancement label Nov 18, 2025

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch 3 times, most recently from 941fc19 to 923b9c5 Compare November 18, 2025 15:42

SergeyKopienko requested a review from Copilot November 18, 2025 15:45

Copilot AI reviewed Nov 18, 2025

View reviewed changes

test/support/utils_sycl_defs.h Outdated Show resolved Hide resolved

test/support/utils_sycl_defs.h Outdated Show resolved Hide resolved

SergeyKopienko requested a review from Copilot November 18, 2025 15:52

Copilot AI reviewed Nov 18, 2025

View reviewed changes

test/support/utils_sycl_defs.h Show resolved Hide resolved

test/support/utils_sycl_defs.h Outdated Show resolved Hide resolved

SergeyKopienko added 2 commits November 18, 2025 16:56

SergeyKopienko force-pushed the dev/skopienko/simplify_tests_new_kernel_name_impl branch from 13d7310 to b5f855d Compare November 18, 2025 15:57

SergeyKopienko added 2 commits November 18, 2025 17:06

test/support/utils_sycl_defs.h - fix review comment: the uniq_kernel_…

b0cc672

…index function should be marked inline to avoid potential ODR (One Definition Rule) violations when this header is included in multiple translation units

test/support/utils_sycl_defs.h - fix review comment: [nitpick] The te…

516b267

…mplate parameter name idx is inconsistent with CallNumber used in other templates in this file

SergeyKopienko requested a review from Copilot November 18, 2025 16:22

Copilot AI reviewed Nov 18, 2025

View reviewed changes

test/support/utils_sycl_defs.h Show resolved Hide resolved

test/support/utils_invoke.h Outdated Show resolved Hide resolved

Update test/support/utils_invoke.h

c636110

Co-authored-by: Copilot <[email protected]>

SergeyKopienko modified the milestones: 2022.12.0, 2022.11.0 Nov 18, 2025

danhoeflinger approved these changes Nov 18, 2025

View reviewed changes

SergeyKopienko merged commit 200710c into main Nov 18, 2025
23 checks passed

SergeyKopienko deleted the dev/skopienko/simplify_tests_new_kernel_name_impl branch November 18, 2025 18:20

Simplify new_kernel_name implementation in tests #2128

Simplify new_kernel_name implementation in tests #2128

Conversation

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

Uh oh!

Uh oh!

danhoeflinger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danhoeflinger commented Mar 13, 2025

Uh oh!

SergeyKopienko commented Mar 13, 2025

Uh oh!

danhoeflinger commented Mar 13, 2025

Uh oh!

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

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

danhoeflinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Simplify `new_kernel_name` implementation in tests #2128

Simplify `new_kernel_name` implementation in tests #2128

SergeyKopienko commented Mar 13, 2025 •

edited

Loading

danhoeflinger left a comment •

edited

Loading

SergeyKopienko commented Mar 13, 2025 •

edited

Loading