Skip to content

[oneTBB] Improve named requirements to better deal with value categories #647

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[oneTBB] Improve named requirements to better deal with value categories #647

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
162fed2
Clarify the use of pseudo-signatures
akukanov Sep 19, 2025
b2431ce
Simplify the definition of ParallelForEachBody requirements
akukanov Sep 19, 2025
2c5ebc0
Improve the definition of ContainerBasedSequence
akukanov Sep 19, 2025
f0cc309
Address the review feedback
akukanov Oct 6, 2025
68826e1
Update the named requirements for reductions to work with rvalues
akukanov Oct 17, 2025
c47c112
Emphasize importance of semantic requirements
akukanov Oct 17, 2025
a940d5c
Add a second signature for the operator with the feeder
akukanov Oct 17, 2025
b724eb5
Improve formatting
akukanov Oct 17, 2025
17fdde7
Improve formatting
akukanov Oct 17, 2025
d549142
Add a table for parameter mapping
akukanov Oct 17, 2025
b13abab
Fix the new table
akukanov Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion source/elements/oneTBB/source/algorithms/functions/parallel_for_each_func.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ Requirements:
* If ``InputIterator`` type does not meet the `Forward Iterator` requirements from the [forward.iterators] section of the ISO C++ Standard,
the ``std::iterator_traits<InputIterator>::value_type`` type must be constructible from ``std::iterator_traits<InputIterator>::reference``.
* The ``Container`` type must meet the :doc:`ContainerBasedSequence requirements <../../named_requirements/algorithms/container_based_sequence>`.
* The type returned by ``Container::begin()`` must meet the same requirements as the ``InputIterator`` type above.
* The type returned by ``std::begin`` and ``std::end`` applied to a ``Container`` object
must meet the same requirements as the ``InputIterator`` type above.

The ``parallel_for_each`` template has two forms.

Expand Down
24 changes: 17 additions & 7 deletions source/elements/oneTBB/source/named_requirements.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,14 @@ an array to be sorted. A type ``T`` would be *sortable* if:

You can write a sorting template function in C++ that sorts an array of any type that is *sortable*.

.. _pseudo_signatures:

Pseudo-Signatures
-----------------

Two approaches for defining named requirements are *valid expressions* and *pseudo-signatures*.
The ISO C++ standard follows the valid *expressions* approach, which shows what the usage pattern looks like for a requirement.
It has the drawback of relegating important details to notational conventions. This document uses
It has the drawback of relegating important details to notational conventions. This document mostly uses
pseudo-signatures because they are concise and can be cut-and-pasted for an initial implementation.

For example, the table below shows pseudo-signatures for a *sortable* type ``T``:
Expand All @@ -43,12 +48,18 @@ For example, the table below shows pseudo-signatures for a *sortable* type ``T``

---------------------------------------------------------------------------------------------

A pseudo-signature describes how an implementation interacts with a type or a function.
A real signature may differ from the pseudo-signature that it implements in ways where implicit
conversions would deal with the difference. For an example type ``U``, the real signature that
implements ``operator<`` in the table above can be expressed as ``int operator<( U x, U y )``,
because C++ permits implicit conversion from ``int`` to ``bool``, and implicit conversion from ``U``
to (``const U&``). Similarly, the real signature ``bool operator<( U& x, U& y )`` is acceptable
because C++ permits implicit addition of a const qualifier to a reference type.
conversions would deal with the difference: function parameters need to implicitly convert
from the ones in the pseudo-signature, and return value needs to implicitly convert to the one
in the pseudo-signature.

For an example type ``U``, the real signature that implements ``operator<`` in the table above
can be expressed as ``int operator<( U x, U y )``, because C++ permits implicit conversion from
``int`` to ``bool``, and implicit conversion from ``const U&`` to ``U`` if the type is copyable.
For a counter-example, the real signature ``bool operator<( U& x, U& y )`` is not acceptable
because C++ does not permit implicit removal of a const qualifier from a type, and so the code
would not compile if the implementation attempts to pass a const object to the function.
Copy link
Contributor

Choose a reason for hiding this comment

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

May be it makes sense to also explicitly mention that if pseudo-signature declares return type as void, but the real signature returns some type, the returned value should be ignored by the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mentioned in an example for swap.


Algorithms
----------
Expand Down Expand Up @@ -81,7 +92,6 @@ Mutexes

Containers
----------

.. toctree::
:titlesonly:

Expand Down
18 changes: 10 additions & 8 deletions ...ements/oneTBB/source/named_requirements/algorithms/container_based_sequence.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,12 @@ ContainerBasedSequence
======================
**[req.container_based_sequence]**

A type `C` satisfies `ContainerBasedSequence` if it meets the following requirements:
A type `C` satisfies `ContainerBasedSequence` if the following expressions are valid
for an object ``c`` of type (possibly ``const``) ``C``:

----------------------------------------------------------------

**ContainerBasedSequence Requirements: Pseudo-Signature, Semantics**

.. note::

In this page ``c`` is an object of type (possibly ``const``) ``C``.

Templates that use the named requirement can impose stricter requirements on the iterator concept.
**ContainerBasedSequence Requirements: Expression, Semantics**

.. cpp:function:: std::begin(c)

Expand All @@ -27,6 +22,13 @@ A type `C` satisfies `ContainerBasedSequence` if it meets the following requirem

Returns an input iterator one past the end of the sequence represented by ``c``.

----------------------------------------------------------------

.. note::

Templates that use ``ContainerBasedSequence`` may impose stricter requirements on the iterator type
returned by ``std::begin``/``std::end``.

See also:

* :doc:`parallel_for_each algorithm <../../algorithms/functions/parallel_for_each_func>`
Expand Down
42 changes: 12 additions & 30 deletions source/elements/oneTBB/source/named_requirements/algorithms/par_for_each_body.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,28 @@ It should also meet one of the following requirements:

**ParallelForEachBody Requirements: Pseudo-Signature, Semantics**

.. cpp:function:: Body::operator()( ItemType item ) const
.. cpp:function:: void Body::operator()( ReferenceType item ) const

Process the received item.

.. cpp:function:: Body::operator()( ItemType item, oneapi::tbb::feeder<ItemType>& feeder ) const
.. cpp:function:: void Body::operator()( ItemType&& item, oneapi::tbb::feeder<ItemType>& feeder ) const
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be ReferenceType as well?

Suggested change
.. cpp:function:: void Body::operator()( ItemType&& item, oneapi::tbb::feeder<ItemType>& feeder ) const
.. cpp:function:: void Body::operator()( ReferenceType item, oneapi::tbb::feeder<ItemType>& feeder ) const

Copy link
Contributor Author

@akukanov akukanov Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This question is related to the question below about "removing" a requirement:

Additional elements submitted into oneapi::tbb::parallel_for_each through the feeder::add are passed to the Body as rvalues. In this case, the corresponding execution of the Body is required to be well-formed.

As I understand it, it says that if the feeder is used, the body has to accept rvalues - because the work elements supplied through the feeder will be "moved" to the body for processing. Since parallel_for_each may only use one operator(), I interpret the requirement as "if your body operator takes a feeder, it has to accept rvalues" - and that is exactly what the pseudo-signature describes.

If the idea is that the original sequence elements are passed just as the result of iterator dereferencing (i.e., *it) while new work elements are "moved" - the operator should then support both ReferenceType (which might be an lvalue reference) and ItemType&& rvalue reference - so it seemingly should take arguments either by value or by const reference. Neither allows modification of the original sequence (which can be fine, given the use of feeder); taking by value will have some copy construction overhead, and taking by const reference makes move useless.

Another option is to use a templated operator with universal references or use different overloads for lvalues and rvalues.

Let's figure out what semantics we want, how it could be implemented in practice, and then hopefully it will be more clear how to describe that in the named requirement.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, the original idea was that the elements of the input sequence are passed as the result of dereferencing. and additional items are always provided as rvalues.

The semantics proposed would break the first part, for example if the iterator have non-const lvalue reference, *it will not be accepted by the body. And the opposite is true, if the operator accepts non-const lvalue reference argument, passing of new items through feeder would not work.

I disagree that parallel_for_each may use only one operator(). I think it should be possible to have several overloads with different first argument types for input sequence and feeder items. By the way, current implementation of oneTBB would also work if the input sequence dereference is passed to the overload with feeder and the additional items are passed to the overload without the feeder:

struct body {
    // overload for feeder items
    void operator()(ItemType&& value_from_feeder) const {}
    
    // overload for input sequence
    void operator()(ItemType& input_sequence, oneapi::tbb::feeder<ItemType>& feeder) const {}
};

Copy link
Contributor Author

@akukanov akukanov Oct 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I disagree that parallel_for_each may use only one operator(). I think it should be possible to have several overloads with different first argument types for input sequence and feeder items.

I meant. only one of the two specified operators - either with or without the feeder, but not both. Several overloads with the feeder would be fine, if there is a good rationale for that.

By the way, current implementation of oneTBB would also work if the input sequence dereference is passed to the overload with feeder and the additional items are passed to the overload without the feeder:

This is simply an incorrect behavior. The choice of using or not using the feeder should not depend on how an item was obtained or on whether we can move it or not. Yes, we can distinguish between these sources of input, but why do we think it matters for the user, based only on the type of the parameter? After all, the feeder can be ignored when provided, but it cannot be created when absent,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The eventual conclusion is that the first parameter should accept both ReferenceType (for values from the input sequence of parallel_for_each) and ItemType&& (for values passed through feeder). It is likely best described by two separate pseudo-signatures, though in some cases a single function can implement both (e.g. with the const ItemType& parameter).

Copy link
Contributor

Choose a reason for hiding this comment

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

This document specify that the body should meet one of the requirements - either be invocable without a feeder or with it.
Should it be specified/recommended which overload should be preferred if both of them are present.
oneTBB implementation prefers the overload with the feeder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think it should be specified. I do not see how the implementation could use both overloads; it would need to decide whether to provide the feeder or not, and how would it decide? So using the overload with the feeder all the time is the "safest" choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For now, I added a note/requirement that exactly one kind of operator() should be provided. It is discussable though.


Process the received item. May invoke the ``feeder.add(T)`` function to spawn additional items.
Process the received item. May invoke the ``feeder.add`` function to spawn additional items.

-----------------------------------------------------------------

.. note::

``ItemType`` may be optionally passed to ``Body::operator()`` by reference.
``const`` and ``volatile`` type qualifiers are also applicable.

Terms
-----

* ``iterator`` determines the type of the iterator passed into the ``parallel_for_each`` algorithm,
which is ``decltype(std::begin(c))`` for the overloads that accept the ``Container`` template argument or ``InputIterator``.
* ``value_type`` - the type ``std::iterator_traits<iterator>::value_type``.
* ``reference`` - the type ``std::iterator_traits<iterator>::reference``.
where ``ItemType`` is ``std::iterator_traits<InputIterator>::value_type`` for the type of the iterator
the ``parallel_for_each`` algorithm operates with, and ``ReferenceType`` is

``oneapi::tbb::parallel_for_each`` requires the ``Body::operator()`` call with an object of the ``reference`` type to be well-formed if
the ``iterator`` meets the `Forward iterator` requirements described in the [forward.iterators] section of the
ISO C++ Standard.
* ``std::iterator_traits<InputIterator>::reference`` if the iterator type is a forward iterator
as described in the [forward.iterators] section of the ISO C++ Standard;
* otherwise, ``ItemType&&``.

`oneapi::tbb::parallel_for_each algorithm <../../algorithms/functions/parallel_for_each_func>`_
requires the ``Body::operator()`` call with an object of type ``const value_type&`` or ``value_type&&`` to be well-formed if following requirements are met:

* the iterator meets the `Input iterator` requirements described in the [input.iterators] section of the ISO C++ Standard
* the iterator does not meet the `Forward iterator` requirements described in the [forward.iterators] section of the ISO C++ Standard

.. caution::

If the ``Body`` only takes non-const lvalue reference to the ``value_type``, the requirements described above
are violated, and the program can be ill-formed.
.. note::

Additional elements submitted into ``oneapi::tbb::parallel_for_each`` through the ``feeder::add`` are passed to the ``Body`` as rvalues. In this case, the corresponding
execution of the ``Body`` is required to be well-formed.
Comment on lines -56 to -57
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it intentional that this requirement is removed?

The usual rules for :ref:`pseudo-signatures <pseudo_signatures>` apply.
Therefore, ``Body::operator()`` may optionally take ``ItemType`` by value.
``const`` and ``volatile`` type qualifiers are also applicable.

See also:

Expand Down
Loading