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.

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

Jump to bottom

[SYCL][Graph] Add exceptions on invalid event and queue usage #250

Merged
merged 5 commits into from
Jul 11, 2023
Merged

[SYCL][Graph] Add exceptions on invalid event and queue usage #250

merged 5 commits into from
Jul 11, 2023

Conversation

Bensuo
Copy link
Collaborator

@Bensuo Bensuo commented Jul 6, 2023 •
edited
Loading

  • Throws when waiting on a queue in recording mode.
  • Throws when waiting on an event from a graph submission.
  • Throws when calling depends_on with an event outside the graph.
  • Adds new tests for the exceptions.

Closes #97

@Bensuo
[SYCL][Graph] Add exceptions on invalid event and queue usage
77816b2 
- Throws when waiting on a queue in recording mode
- Throws when waiting on an event from a graph submission
- Throws when calling depends_on with an event outside the graph
- Add tests for these exceptions
@Bensuo Bensuo added the Graph Implementation Related to DPC++ implementation and testing label Jul 6, 2023
EwanC
EwanC approved these changes Jul 6, 2023
View reviewed changes
Copy link
Collaborator

@EwanC EwanC left a comment

Choose a reason for hiding this comment

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

LGTM, one abi test that needs updated

sycl/include/sycl/handler.hpp Show resolved Hide resolved
mfrancepillois
mfrancepillois approved these changes Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@mfrancepillois mfrancepillois left a comment

Choose a reason for hiding this comment

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

LGTM, but the tests could be improved to avoid silent failures

sycl/test-e2e/Graph/invalid_event_wait.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/Graph/invalid_queue_wait.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/Graph/invalid_depends_on.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/Graph/invalid_depends_on.cpp Outdated Show resolved Hide resolved
julianmi
julianmi approved these changes Jul 7, 2023
View reviewed changes
Copy link
Collaborator

@julianmi julianmi left a comment

Choose a reason for hiding this comment

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

LGTM. We could extend the unittests as well to check for the exceptions introduced.

@Bensuo Bensuo merged commit 7d88887 into sycl-graph-develop Jul 11, 2023
@Bensuo Bensuo deleted the ben/invalid-event-queue branch July 11, 2023 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EwanC EwanC EwanC approved these changes

@mfrancepillois mfrancepillois mfrancepillois approved these changes

@julianmi julianmi julianmi approved these changes

@reble reble Awaiting requested review from reble

Assignees
No one assigned
Labels
Graph Implementation Related to DPC++ implementation and testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement error handling for invalid event/queue usage
4 participants
@Bensuo @EwanC @julianmi @mfrancepillois