Skip to content

[SYCL] Use secondary queue in assert post-processing #4759

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

Merged

Conversation

steffenlarsen
Copy link
Contributor

@steffenlarsen steffenlarsen commented Oct 14, 2021

For submissions with a secondary queue the post-process lambda currently captures the secondary queue but does not pass it along. These changes uses the secondary queue if it is not a host queue.

Note: This should fix the current post-commit failures.

For submissions with a secondary queue the post-process lambda currently
captures the secondary queue but does not pass it along. These changes
uses the secondary queue if it is not a host queue.

Signed-off-by: Steffen Larsen <[email protected]>
@steffenlarsen steffenlarsen requested a review from a team as a code owner October 14, 2021 11:16
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

LGTM.

Though, I wonder how this broke smth after merge.

@bader
Copy link
Contributor

bader commented Oct 14, 2021

LGTM.

Though, I wonder how this broke smth after merge.

Pre-commit doesn't cover the build with disabled assertions.

@steffenlarsen
Copy link
Contributor Author

LGTM.

Though, I wonder how this broke smth after merge.

The submit template with a secondary queue doesn't seem to have instantiated in testing, so the warning for unused lambda capture wasn't issued. #4744 added test that use secondary queue submits, so the warning was manifested.

@bader bader merged commit 7b8f3b5 into intel:sycl Oct 14, 2021
@steffenlarsen steffenlarsen deleted the steffen/queue_submit_secondary_fallback_assert branch December 6, 2023 11:38
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.

4 participants