-
Notifications
You must be signed in to change notification settings - Fork 787
[SYCL][Graph] Make SYCL-Graph functions thread-safe #10778
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
Conversation
* [SYCL][Graph] Makes command graph functions thread-safe Addresses comments made on the first PR commit. Mutexes are now added to Graph implementation entry points instead of end points as was the case in the previous commit. Adds "build_pthread_inc" lit test macro to facilitate the compilation of the threading tests. Removes std::barrier (std-20) dependency in threading tests. Addresses Issue: #85 * [SYCL][Graph] Makes command graph functions thread-safe Moves threading tests that do not require a device to run to unitests * Update sycl/source/detail/graph_impl.cpp Co-authored-by: Ben Tracy <[email protected]> * [SYCL][Graph] Makes command graph functions thread-safe Adds some comments. * Update sycl/source/handler.cpp Co-authored-by: Pablo Reble <[email protected]> * Update sycl/source/detail/graph_impl.hpp Co-authored-by: Ewan Crawford <[email protected]> * [SYCL][Graph] Makes command graph functions thread-safe Adds dedidacted sub-class to unitests for multi-threading unitests * [SYCL][Graph] Makes command graph functions thread-safe Adds comments * [SYCL][Graph] thread-safe: bug fix after rebase --------- Co-authored-by: Ben Tracy <[email protected]> Co-authored-by: Pablo Reble <[email protected]> Co-authored-by: Ewan Crawford <[email protected]>
Removes the test-e2e dependency to graph_impl.hpp by changing the e2e test to an unitests.
Marked this as draft again, as we have discovered that graph enqueue isn't quite thread-safe yet |
This reverts commit 09c1af5.
Fixes memory leak issues that occurred during multithreads graph finalization
This issue has now been resolved, so PR is Open and ready for review from @intel/llvm-reviewers-runtime |
Co-authored-by: Steffen Larsen <[email protected]>
…this object (#302) The way exec_graph_impl is used and the C++ semantic prevent concurrent access the destructor of this object.
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.
LGTM!
This PR makes the new APIs defined by sycl_ext_oneapi_graph thread safe.
Authors
Co-authored-by: Pablo Reble [email protected]
Co-authored-by: Julian Miller [email protected]
Co-authored-by: Ben Tracy [email protected]
Co-authored-by: Ewan Crawford [email protected]
Co-authored-by: Maxime France-Pillois [email protected]