Skip to content

[SYCL] Stop throwing exception when passing empty accessor to handler::require() #10597

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

maarquitos14
Copy link
Contributor

According to the changes made in https://github.com/KhronosGroup/SYCL-Docs/pull/434/files, empty accessors can now be passed to handler::require() without throwing.

@maarquitos14 maarquitos14 requested a review from a team as a code owner July 27, 2023 09:30
Signed-off-by: Maronas, Marcos <[email protected]>
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 09:43 — with GitHub Actions Inactive
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 10:27 — with GitHub Actions Inactive
Signed-off-by: Maronas, Marcos <[email protected]>
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 11:03 — with GitHub Actions Inactive
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 11:58 — with GitHub Actions Inactive
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 12:37 — with GitHub Actions Inactive
@maarquitos14 maarquitos14 temporarily deployed to aws July 27, 2023 13:16 — with GitHub Actions Inactive
@@ -818,20 +818,18 @@ int main() {
}
}

// SYCL2020 4.9.4.1: calling require() on empty accessor should throw
// SYCL2020 4.9.4.1: calling require() on empty accessor should not throw.
Copy link
Contributor

@cperkinsintel cperkinsintel Jul 27, 2023

Choose a reason for hiding this comment

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

I'm confused. This is exactly wrong. The spec at 4.9.4.1 table 131 first entry says:

Throws exception with the
errc::invalid error code if
(acc.empty() == true).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/KhronosGroup/SYCL-Docs/pull/434/files in this PR, it was updated to say it's allowed.

Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

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

LGTM

@steffenlarsen steffenlarsen merged commit 7bf1f57 into intel:sycl Jul 28, 2023
mdtoguchi pushed a commit to mdtoguchi/llvm that referenced this pull request Oct 18, 2023
…::require() (intel#10597)

According to the changes made in
https://github.com/KhronosGroup/SYCL-Docs/pull/434/files, empty
accessors can now be passed to `handler::require()` without throwing.

---------

Signed-off-by: Maronas, Marcos <[email protected]>
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