Skip to content

Extend test coverage for different policy qualifiers #2102

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

Draft
wants to merge 55 commits into
base: main
Choose a base branch
from

Conversation

SergeyKopienko
Copy link
Contributor

@SergeyKopienko SergeyKopienko commented Mar 5, 2025

In this PR we extend test coverage to detect compile-time errors in oneDPL code like described at #2041.
We extend test coverage under false-run-time condition check so we only compile additional Kernels without their runs.
This idea was proposed by @danhoeflinger.

Also in this PR we modified a lot of tests.
The goal - to pass temporary created execution policies inside oneDPL algorithms as l-value / r-value depends on qualifiers of execution policy passed from test environment. This mandatory needed for detect compile errors ...mangled name already used... inside oneDPL Kernel's code.

Implementation details.

To achieve this goal we implements two things in tests:

  1. Create helper for return temporary execution policy as l-value / r-value :
// struct policy_container - a container for policy which return saved policy
// as l-value or r-value depends on source policy type qualifiers
template <typename _PolicySource, typename _PolicyNew>
struct policy_container
{
    using _PolicyNewDecayed = std::decay_t<_PolicyNew>;

    _PolicyNewDecayed __policy_new;

    policy_container(_PolicyNewDecayed&& __policy_new) : __policy_new(std::move(__policy_new))
    {
    }

    // Delete copy constructor to avoid copying of policy_container
    policy_container(const policy_container&) = delete;

    // Delete assignment operator to avoid copying of policy_container
    policy_container&
    operator=(const policy_container&) = delete;

    using TestingPolicyType = std::conditional_t<
        std::is_reference_v<_PolicySource>,
        std::conditional_t<std::is_rvalue_reference_v<_PolicySource>, _PolicyNewDecayed&&, const _PolicyNewDecayed&>,
        _PolicyNewDecayed>;

    // Return testing policy
    TestingPolicyType get()
    {
        return static_cast<TestingPolicyType>(__policy_new);
    }
};
  1. Create new CREATE_NEW_POLICY macros to simplify creation and passing into calling algorithm new temporary execution policy:
// Create copy of hetero policy with new name (appends idx) and pass copied instance as l-value / r-value
// depends on qualifiers of source policy type.
// The source policy state is not changed, so it can be used in the same test
// ATTENTION: new Kernel name generation depends on TEST_EXPLICIT_KERNEL_NAMES macro state
#define CREATE_NEW_POLICY(policy_src, idx)                                                                             \
        TestUtils::policy_container<                                                                                   \
            decltype(policy_src),                                                                                      \
            decltype(TestUtils::make_new_policy<TestUtils::new_kernel_name<decltype(policy_src), idx>>(policy_src))    \
        >(                                                                                                             \
            TestUtils::make_new_policy<TestUtils::new_kernel_name<decltype(policy_src), idx>>(policy_src)              \
         ).get()

// Create copy of hetero policy with new name and pass copied instance as l-value / r-value
// depends on qualifiers of source policy type.
// The source policy state is not changed, so it can be used in the same test.
#define CREATE_NEW_POLICY_WITH_NAME(policy_src, NewKernelName)                                                         \
        TestUtils::policy_container<                                                                                   \
            decltype(policy_src),                                                                                      \
            decltype(TestUtils::make_new_policy<NewKernelName>(policy_src))                                            \
        >(                                                                                                             \
            TestUtils::make_new_policy<NewKernelName>(policy_src)                                                      \
         ).get()

// Clone policy and pass cloned instance as l-value / r-value
// depends on qualifiers of source policy type.
#define CLONE_NEW_POLICY(policy_src)                                                                                   \
        TestUtils::policy_container<                                                                                   \
            decltype(policy_src),                                                                                      \
            decltype(policy_src)                                                                                       \
        >(                                                                                                             \
            policy_src                                                                                                 \
         ).get()

As result, now in oneDPL tests we are simplified a lot of code:

  • before:
Iterator result = ::std::adjacent_find(make_new_policy<new_kernel_name<Policy, 0>>(exec), first, last, comp);

in this code we passed new temporary execution policy into oneDPL algorithm as ExecutionPolicy&& (r-value) and it's hide a lot of compile errors in oneDPL Kernels when we going to move execution policy into oneDPL algorithms in test environment.

  • now:
Iterator result = ::std::adjacent_find(CREATE_NEW_POLICY(exec, 0), first, last, comp);

in this code we passed new temporary execution policy into oneDPL algorithm with qualifiers which depends on source execution policy (exec) type qualifiers (l-value / r-value).

@SergeyKopienko SergeyKopienko requested a review from mmichel11 March 5, 2025 17:05
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch 2 times, most recently from 1929dc4 to 7e76a2a Compare March 5, 2025 17:29
@mmichel11
Copy link
Contributor

The approach looks good to me to detect these sorts of issues assuming the compiler does not optimize out kernel compilation due to the if (false).

@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch 3 times, most recently from 736f5db to 1ce4b3c Compare March 13, 2025 10:53
@SergeyKopienko SergeyKopienko requested a review from rarutyun March 13, 2025 11:22
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch 4 times, most recently from 14aeb4a to 717584a Compare March 20, 2025 09:55
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch from 717584a to 580eeba Compare March 27, 2025 18:50
@SergeyKopienko SergeyKopienko marked this pull request as draft April 1, 2025 08:04
@SergeyKopienko SergeyKopienko modified the milestones: 2022.9.0, 2022.10.0 Apr 3, 2025
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch from ecca23e to 10fa5a2 Compare May 8, 2025 15:49
…or both cases - when TEST_DPCPP_BACKEND_PRESENT is defined and when TEST_DPCPP_BACKEND_PRESENT isn't defined
…d declaration of struct / class in Kernel names
…ompile_checker and create create_compile_checker(_ExecutionPolicy&& policy) function
…ompile_checker and create create_compile_checker(_ExecutionPolicy&& policy) function - apply in tests
…parameter of test_with_usm functions spezialization
…ass.cpp - fix compile error: avoid forward declaration of struct / class in Kernel names
…x compile error: forgot to specify policy type in template parameters
…x compile error: avoid forward declaration of struct / class in Kernel names
…rs: extract lambdas from template code which depends on execution policy type
…mpile error: avoid forward declaration of struct / class in Kernel names
…void forward declaration of struct / class in Kernel names
…ompile error: avoid forward declaration of struct / class in Kernel names
… CREATE_NEW_POLICY_WITH_NAME + check compilation
@SergeyKopienko SergeyKopienko force-pushed the dev/skopienko/extend_test_coverage_for_diff_policy_qualifiers branch from 0c278e8 to 763c97e Compare June 18, 2025 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement test Test only Change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants