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] oneDPL '__future' class code cleanup and API fix #1962

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
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
66 changes: 38 additions & 28 deletions include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -701,35 +701,41 @@ struct __deferrable_mode
{
};

//A contract for future class: <sycl::event or other event, a value, sycl::buffers..., or __usm_host_or_buffer_storage>
//Impl details: inheritance (private) instead of aggregation for enabling the empty base optimization.
template <typename _Event, typename... _Args>
class __future : private std::tuple<_Args...>
// An overload of __wait_and_get_value for 'sycl::buffer'
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we modify these function without move them outside?

Copy link
Contributor Author

@MikeDvorskiy MikeDvorskiy Dec 9, 2024

Choose a reason for hiding this comment

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

The change also solves a code coupling issue.
There was a code coupling between following types: __future, 'sycl::buffer', '__result_and_scratch_storage'.

Copy link
Contributor

@SergeyKopienko SergeyKopienko Dec 9, 2024

Choose a reason for hiding this comment

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

But why should move them outside of class __future ?
Let's still use them as class-member functions, why not?

template <typename _Event, typename _T>
constexpr auto
__wait_and_get_value(_Event&&, const sycl::buffer<_T>& __buf, std::size_t __idx = 0)
{
_Event __my_event;
//according to a contract, returned value is one-element sycl::buffer
return __buf.get_host_access(sycl::read_only)[__idx];
}

template <typename _T>
constexpr auto
__wait_and_get_value(const sycl::buffer<_T>& __buf)
{
//according to a contract, returned value is one-element sycl::buffer
return __buf.get_host_access(sycl::read_only)[0];
}
// An overload of __wait_and_get_value for '__result_and_scratch_storage'
template <typename _Event, typename _ExecutionPolicy, typename _T>
constexpr auto
__wait_and_get_value(_Event&& __event, const __result_and_scratch_storage<_ExecutionPolicy, _T>& __storage,
std::size_t __idx = 0)
{
return __storage.__wait_and_get_value(__event, __idx);
}

template <typename _ExecutionPolicy, typename _T>
constexpr auto
__wait_and_get_value(const __result_and_scratch_storage<_ExecutionPolicy, _T>& __storage)
{
return __storage.__wait_and_get_value(__my_event);
}
template <typename _Event, typename _T>
constexpr auto
__wait_and_get_value(_Event&& __event, const _T& __val, std::size_t)
{
__event.wait_and_throw();
return __val;
}

template <typename _T>
constexpr auto
__wait_and_get_value(const _T& __val)
{
wait();
return __val;
}
//A contract for 'future' class:
//* The first argument is an event, which has 'wait_and_throw' method
//* The second and the following argument a trivial type T or RAII storage.
//* The 'future' class extends the lifetime for such RAII objects.
//* Impl details: inheritance (private) instead of aggregation for enabling the empty base optimization.
template <typename _Event, typename... _Args>
class __future : private std::tuple<_Args...>
{
_Event __my_event;

public:
__future(_Event __e, _Args... __args) : std::tuple<_Args...>(__args...), __my_event(__e) {}
Expand Down Expand Up @@ -764,13 +770,17 @@ class __future : private std::tuple<_Args...>
#endif
}

//_ArgsIdx specifies a compile time index of i-th argument passed into '__future' constructor after an event
//__elem_idx specifies a runtime time index of k-th element of i-th argument in case when i-th argument
// is not scalar value - an array/buffer like type.
template <std::size_t _ArgsIdx = 0>
auto
get()
get(std::size_t __elem_idx = 0)
{
if constexpr (sizeof...(_Args) > 0)
{
auto& __val = std::get<0>(*this);
return __wait_and_get_value(__val);
auto& __val = std::get<_ArgsIdx>(*this);
return __wait_and_get_value(event(), __val, __elem_idx);
}
else
wait();
Expand Down
Loading