Skip to content
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

[oneDPL] Create a subsection for is_execution_policy type trait #567

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
61 changes: 54 additions & 7 deletions source/elements/oneDPL/source/parallel_api/execution_policies.rst
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ Execution Policies
C++ Standard Aligned Execution Policies
+++++++++++++++++++++++++++++++++++++++

oneDPL has the set of execution policies and related utilities that are semantically aligned
with the `C++ Standard`_, 6th edition (C++20):
oneDPL has the set of execution policies semantically aligned with the `C++ Standard`_, 6th edition (C++20):

.. code:: cpp

Expand All @@ -30,17 +29,16 @@ with the `C++ Standard`_, 6th edition (C++20):
inline constexpr parallel_unsequenced_policy par_unseq { /*unspecified*/ };
inline constexpr unsequenced_policy unseq { /*unspecified*/ };

template <class T>
struct is_execution_policy;

template <class T>
inline constexpr bool is_execution_policy_v = oneapi::dpl::execution::is_execution_policy<T>::value;
}
}
}

See "Execution policies" in the `C++ Standard`_ for more information.

.. note::
The ``std::is_execution_policy`` type trait resolves to ``std::false_type`` for oneDPL execution policies.
Implementations and programs should instead use the :ref:`oneDPL type trait <exec-policy-type-trait>`.

Device Execution Policy
+++++++++++++++++++++++

Expand Down Expand Up @@ -180,5 +178,54 @@ as the template argument, otherwise unspecified.
Return a policy object constructed from ``policy``, with a new kernel name provided as the template
argument. If no policy object is provided, the new policy is constructed from ``dpcpp_default``.

.. _exec-policy-type-trait:

Execution Policy Type Trait
+++++++++++++++++++++++++++

oneDPL provides type trait utilities to detect its execution policy types at compile time for the purpose of
function overload resolution:

.. code:: cpp

// Defined in <oneapi/dpl/execution>

namespace oneapi {
namespace dpl {
danhoeflinger marked this conversation as resolved.
Show resolved Hide resolved

template <class T>
struct is_execution_policy { /*see below*/ };

template <class T>
constexpr bool is_execution_policy_v = oneapi::dpl::is_execution_policy<T>::value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constexpr bool is_execution_policy_v = oneapi::dpl::is_execution_policy<T>::value;
inline constexpr bool is_execution_policy_v = oneapi::dpl::is_execution_policy<T>::value;

Copy link
Contributor Author

@akukanov akukanov Oct 9, 2024

Choose a reason for hiding this comment

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

It's a template, inline should not be needed. See the standard (https://eel.is/c++draft/execution.syn):

namespace std {
  // [execpol.type], execution policy type trait
  template<class T> struct is_execution_policy;
  template<class T> constexpr bool is_execution_policy_v = is_execution_policy<T>::value;
}

Copy link
Contributor Author

@akukanov akukanov Oct 9, 2024

Choose a reason for hiding this comment

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

OK, seems inline is necessary for C++17/20. Thanks, I will get it back.


namespace execution {

template <class T>
struct is_execution_policy { /*see below*/ };

template <class T>
constexpr bool is_execution_policy_v = oneapi::dpl::execution::is_execution_policy<T>::value;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
constexpr bool is_execution_policy_v = oneapi::dpl::execution::is_execution_policy<T>::value;
inline constexpr bool is_execution_policy_v = oneapi::dpl::execution::is_execution_policy<T>::value;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here.


}
}
}

``oneapi::dpl::is_execution_policy`` and ``oneapi::dpl::execution::is_execution_policy`` must be treated as name aliases
to the same class template. It is unspecified in which namespace the underlying class template and its specializations
are defined.
Comment on lines +215 to +216
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
to the same class template. It is unspecified in which namespace the underlying class template and its specializations
are defined.
to the same class template. It is unspecified, which namespace the underlying class template and its specializations
are defined in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Spellcheckers do not complain :), so I will keep this intact.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, this is small suggestion. As far as I understand "preposition + which" is consider to be poorer English. More common use is "bla-bla, which + <proposition at the end>. Again, not proposing to change that but for my curiosity we could summon a native speaker. @danhoeflinger, could you please share your opinion?


.. note::
The ``oneapi::dpl::execution::is_execution_policy`` class originated in the oneDPL specification version 1.0,
while ``oneapi::dpl::is_execution_policy`` has been added later to better align with the C++ standard.

For writing new code, use of the type trait utilities in ``namespace oneapi::dpl`` is strongly recommended. Those
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
For writing new code, use of the type trait utilities in ``namespace oneapi::dpl`` is strongly recommended. Those
For a new code, use of the type trait utilities in ``namespace oneapi::dpl`` is highly encouraged. Those

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a similar change.

in ``namespace oneapi::dpl::execution`` are provided for backward compatibility and may be deprecated in the future.

``is_execution_policy<T>`` must have the characteristics of ``std::true_type`` if ``T`` is one of the above specified or
implementation-defined oneDPL execution policy types, otherwise it must have the characteristics of ``std::false_type``.
Comment on lines +225 to +226
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of this super generic and kind of vague wording?

I specifically mean "is one of the above specified or implementation-defined oneDPL execution policy types".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Compare with the C++ standard wording:

is_execution_policy<T> is a Cpp17UnaryTypeTrait with a base characteristic of true_type if T is the type of a standard or implementation-defined execution policy, otherwise false_type.

It is assumed that an implementation can define additional policy types, for which the trait also resolves to true_type.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Given that this is a specification, we allow the implementations of oneDPL spec to have their own implementation-defined execution policies, if they will.

Thanks.

Following the C++ Standard, ``is_execution_policy<T>`` does not automatically strip references and cv-qualifiers from
its template argument. [*Note*: Use it with ``std::decay_t<T>`` or similar type transformation utilities. -- *end note*]

.. _`C++ Standard`: https://isocpp.org/std/the-standard
.. _`SYCL`: https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html