-
Notifications
You must be signed in to change notification settings - Fork 115
Remove the usages of __future
on internal layers
#2261
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
base: main
Are you sure you want to change the base?
Conversation
@danhoeflinger, I think you may continue thinks about re-design of |
e876ce8
to
fe1a183
Compare
…remove __future::__make_future as not required anymore
# This is the 1st commit message: Move out __checked_deferrable_wait() from __future # The commit message #2 will be skipped: # Move out __checked_deferrable_wait() from __future
…the type of result for __parallel_stable_sort + __device_backend_tag
…fix .wait() calls
…ra local variables
…fix compile error
…remove __future::__wait_and_get_value(const __result_and_scratch_storage_base_ptr& __p_storage) as unused anymore
fe1a183
to
865f81d
Compare
…fix on caller side
First, I have a couple of questions:
|
|
d42f486
to
6a1f61a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the internal usage of the __future type by replacing it with tuple-based returns or direct event returns throughout the oneDPL hetero backend. The changes update various algorithm implementations and backend utilities so that asynchronous kernel submissions now return a std::tuple or sycl::event instead of a __future.
- Removed __future wrapping and replaced with std::tuple<sycl::event, storage> patterns.
- Updated return types and related documentation/comments in several hetero backend files.
- Switched shared_ptr types to the new __result_and_scratch_storage_base_ptr for memory lifetime management.
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
File | Description |
---|---|
include/oneapi/dpl/pstl/hetero/numeric_ranges_impl_hetero.h | Updated __parallel_transform_reduce call to return a tuple and use __wait_and_get_value. |
include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h | Removed internal __future methods and updated wait implementations. |
include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_merge_sort.h | Changed shared_ptr usage to __result_and_scratch_storage_base_ptr. |
... | Other related files updated to use tuple returns or direct sycl::event. |
Comments suppressed due to low confidence (3)
include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_utils.h:736
- Update the inline documentation for __wait_and_get_value to reflect that it is now used in tandem with the new __result_and_scratch_storage_base_ptr based return pattern instead of __future.
std::pair<std::size_t, std::size_t> __wait_and_get_value(const std::shared_ptr<__result_and_scratch_storage_base>& __p_storage)
include/oneapi/dpl/pstl/hetero/dpcpp/parallel_backend_sycl_merge_sort.h:591
- Ensure that the naming convention for the new type __result_and_scratch_storage_base_ptr is applied consistently throughout the file after replacing shared_ptr, to avoid confusion in the API.
std::shared_ptr<__result_and_scratch_storage_base> __p_result_and_scratch_storage_base;
include/oneapi/dpl/pstl/hetero/numeric_ranges_impl_hetero.h:60
- Confirm that using __storage.__wait_and_get_value(__event) in place of the previous get() call maintains the expected asynchronous behavior without introducing unintended blocking, and that it integrates seamlessly with the tuple-based return conventions.
return __storage.__wait_and_get_value(__event);
In this PR we remove the usage of
__future
class on internal layers ofoneDPL
code.Instead of them now we using:
std::tuple<sycl::event>
;std::tuple<sycl::event, __result_and_scratch_storage>
;std::tuple<sycl::event, __result_and_scratch_storage_base_ptr>
.These types are enough for implementation of required functional.
Now we create
__future
instances only in experimentalasync
algorithm implementations.Also in this PR we introduced new type
__result_and_scratch_storage_base_ptr
and declare it asusing __result_and_scratch_storage_base_ptr = std::unique_ptr<__result_and_scratch_storage_base>;
So now we are using
__result_and_scratch_storage_base_ptr
type instead ofstd::shared_ptr<__result_and_scratch_storage_base>
in all code.