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

[SYCL][Graphs] Error checking improvements and fixes for SYCL-Graphs #10628

Closed
wants to merge 8 commits into from

Conversation

Bensuo
Copy link
Contributor

@Bensuo Bensuo commented Jul 31, 2023

Error checking improvements and fixes for SYCL-Graphs

This PR includes various fixes and improvements to error handling to better align the SYCL-Graphs implementation with the specification: https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/proposed/sycl_ext_oneapi_graph.asciidoc

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]

Bensuo added 2 commits July 31, 2023 16:59
- make_edge now checks for cycles
- no_cycle_check property can now be passed to skip them
- Various other error checks in make_edge
- Generic depth first search mechanism added to graph_impl
- New e2e tests for cycle checks
- Unit tests for other basic errors
- Prevent adding duplicate edges
- Adds testing for the graph structure after a cycle error is caught to ensure it is unchanged.
- Skip cycle checks when dst has no successors
- Error when we detected immediate command lists
- Throws exception with sycl::invalid
- Test which uses property::queue::<no_>immediate_command_list to test errors.
@Bensuo Bensuo temporarily deployed to aws July 31, 2023 17:00 — with GitHub Actions Inactive
@Bensuo Bensuo temporarily deployed to aws July 31, 2023 17:51 — with GitHub Actions Inactive
…ording a queue (#283)

Factorizes the exception throwing method when the explicit API is used on a graph recording a queue
Improves the test while_recording to test throwing an invalid exception for the two explicit graph:add entry points.

Addresses issue #271

* Update sycl/source/detail/graph_impl.hpp

Co-authored-by: Ben Tracy <[email protected]>

* Update sycl/test-e2e/Graph/Explicit/while_recording.cpp

Co-authored-by: Ben Tracy <[email protected]>

* Update sycl/source/detail/graph_impl.hpp

Co-authored-by: Ewan Crawford <[email protected]>

---------

Co-authored-by: Ben Tracy <[email protected]>
Co-authored-by: Ewan Crawford <[email protected]>
@mfrancepillois mfrancepillois force-pushed the sycl-graphs-improvements-fixes branch from f0537a3 to 4b980d0 Compare August 3, 2023 10:54
@mfrancepillois mfrancepillois temporarily deployed to aws August 3, 2023 11:05 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 3, 2023 11:50 — with GitHub Actions Inactive
* [SYCL][Graph] Implement exceptions for incompatible extensions

Throws an invalid exception when trying to use the following extensions
along with Graph.

- sycl_ext_oneapi_enqueue_barrier
- sycl_ext_oneapi_memcpy2d
- sycl_ext_codeplay_kernel_fusion
- sycl_ext_oneapi_kernel_properties
- sycl_ext_oneapi_device_global

Closes Issue: #154

* [SYCL][Graph] Implement exceptions for incompatible extensions

Adds info to exception message
Moves tests from e2e to unitests when possible

* [SYCL][Graph] Implement exceptions for incompatible extensions

Corrects some typos and adds comments.

* [SYCL][Graph] Implement exceptions for incompatible extensions

Used a template function to throw exception instead of a parametrized function.

* [SYCL][Graph] Implement exceptions for incompatible extensions

Moves Sycl-extension enum definition.
Limits graph recording to non-explicit path in the new tests.

* [SYCL][Graph] Implement exceptions for incompatible extensions

Updates Linux ABI dump file with the new handler function throwing exception.
@mfrancepillois mfrancepillois temporarily deployed to aws August 3, 2023 15:08 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 3, 2023 15:54 — with GitHub Actions Inactive
* [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]>
@mfrancepillois mfrancepillois force-pushed the sycl-graphs-improvements-fixes branch from 9a5f377 to bfcd551 Compare August 4, 2023 14:47
@mfrancepillois mfrancepillois temporarily deployed to aws August 4, 2023 14:58 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 4, 2023 15:39 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 7, 2023 13:41 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 7, 2023 14:58 — with GitHub Actions Inactive
Removes the test-e2e dependency to graph_impl.hpp by changing the e2e test
to an unitests.
@mfrancepillois mfrancepillois force-pushed the sycl-graphs-improvements-fixes branch from 6af80ab to e578f69 Compare August 7, 2023 16:43
@mfrancepillois mfrancepillois temporarily deployed to aws August 7, 2023 16:55 — with GitHub Actions Inactive
@mfrancepillois mfrancepillois temporarily deployed to aws August 7, 2023 17:58 — with GitHub Actions Inactive
mfrancepillois and others added 2 commits August 9, 2023 15:13
…kend (#280)

* [SYCL][Graph] Throw exception when creating graph for unsupported backend

- Checks backend when creating graphs and throws an exception is the backend is not supported.
- Adds an e2e test to verify this exception throwing.
- Updates some comments
- Improves mock usage in Unitest to avoid having to force emulation mode

---------

Co-authored-by: Pablo Reble <[email protected]>
Co-authored-by: Julian Miller <[email protected]>
* [SYCL][Graph] Makes command graph functions thread-safe (bugfix)

Removes the test-e2e dependency to graph_impl.hpp by changing the e2e test
to an unitests.

* [SYCL][Graph] Error on invalid buffer behaviour

- Error for buffers with write back enabled.
- Error for host accessors to buffers in use by a graph.
- Unit tests added for these cases
- Fixed E2E tests which were not compliant with these restrictions.

* [SYCL][Graph] Implement assume_data_outlives_buffer property

- Add assume_data_outlives_buffer property
- Add check to disallow buffers with host data without this property
- Add a generic test for this.
- Update E2E tests which now require the property

* Update symbol dumps

* [SYCL][Graph] Implement assume_buffer_outlives_graph property and checks

* [SYCL][Graph] Add E2E test for assume_buffer_outlives_graph property

* [SYCL][Graph] Fix circular reference between queue and graph_impl

- Use weak_ptrs in both classes to prevent circular references and unnecessary lifetime extensions
- Update unit tests for new properties

* [SYCL][Graph] Remove unnecessary test line

* [SYCL][Graph] Make graph use count in memobjects atomic

* [SYCL][Graph] Make markNoLongerBeingUsedInGraph() thread safe

---------

Co-authored-by: Maxime France-Pillois <[email protected]>
Co-authored-by: Ewan Crawford <[email protected]>
@EwanC EwanC closed this Aug 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants