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

Fix contradiction with context constructors #628

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

gmlueck
Copy link
Contributor

@gmlueck gmlueck commented Sep 20, 2024

The context constructor overloads in the context synopsis did not match the description in the text of the specification. We decided that the overloads in the synopsis were what we intended, so change the text to match.

We also discovered that two constructor overloads taking a platform argument were present in SYCL 1.2.1 and were inadvertently dropped from the SYCL 2020 specification. Add them back.

Clarify the behavior of the constructors when the set of devices is empty. We decided that this should throw an exception, which is consistent with OpenCL's clCreateContext.

Closes #470
Closes #474

@gmlueck
Copy link
Contributor Author

gmlueck commented Sep 20, 2024

The PR is stacked on top of #587. I'll retarget it once #587 is merged.

EDIT: #587 is merged now, and I've retargeted this PR to the "main" branch.

@tomdeakin
Copy link
Contributor

Retarget to main branch.

@gmlueck gmlueck changed the base branch from gmlueck/reformat-context to main December 5, 2024 19:11
The context constructor overloads in the `context` synopsis did not
match the description in the text of the specification.  We decided that
the overloads in the synopsis were what we intended, so change the text
to match.

We also discovered that two constructor overloads taking a `platform`
argument were present in SYCL 1.2.1 and were inadvertently dropped from
the SYCL 2020 specification.  Add them back.

Clarify the behavior of the constructors when the set of devices is
empty.  We decided that this should throw an exception, which is
consistent with OpenCL's `clCreateContext`.

Closes KhronosGroup#470
Closes KhronosGroup#474
@gmlueck gmlueck force-pushed the gmlueck/context-constructors branch from 29cbcdc to a4543c6 Compare December 5, 2024 19:15
@TApplencourt TApplencourt self-requested a review January 9, 2025 16:15
----
explicit context(const device& dev, async_handler asyncHandler = {})
explicit context(const platform &plt, const property_list &propList = {})
Copy link
Member

Choose a reason for hiding this comment

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

Have we changed the coding style about * and &? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, thanks for catching this. Fixed in 2887f9c.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants