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] Support sycl_khr_default_context #15645

Draft
wants to merge 2 commits into
base: sycl
Choose a base branch
from

Conversation

KornevNikita
Copy link
Contributor

Spec: KhronosGroup/SYCL-Docs#624
sycl-cts: KhronosGroup/SYCL-CTS#960

This patch is intended to be merged only when there are sycl-cts tests and the ratified khr extension.

Spec: KhronosGroup/SYCL-Docs#624
sycl-cts: KhronosGroup/SYCL-CTS#960

This patch is intended to be merged only when there are sycl-cts tests
and the ratified khr extension.
@gmlueck
Copy link
Contributor

gmlueck commented Oct 10, 2024

sycl-cts: KhronosGroup/SYCL-CTS#960

This patch is intended to be merged only when there are sycl-cts tests and the ratified khr extension.

It looks like those tests are for the oneAPI extension ext_oneapi_get_default_context not for the KHR extension khr_get_default_context. We should have tests for the KHR extension.

@KornevNikita
Copy link
Contributor Author

It looks like those tests are for the oneAPI extension ext_oneapi_get_default_context not for the KHR extension khr_get_default_context. We should have tests for the KHR extension.

@gmlueck as far as I understand you said that there is no sense to have sycl_ext_oneapi_default_context tests and we should add sycl_khr_oneapi_default_context instead. I thought Michael will update his PR to reflect it. Am I wrong? @0x12CC

@gmlueck
Copy link
Contributor

gmlueck commented Oct 10, 2024

@gmlueck as far as I understand you said that there is no sense to have sycl_ext_oneapi_default_context tests and we should add sycl_khr_oneapi_default_context instead. I thought Michael will update his PR to reflect it. Am I wrong? @0x12CC

Oh, I see. If @0x12CC is going to update KhronosGroup/SYCL-CTS#960, then everything is good.

@0x12CC
Copy link
Contributor

0x12CC commented Oct 12, 2024

Sorry, I missed this discussion earlier. I've updated KhronosGroup/SYCL-CTS#960 to test the KHR extension instead.

@gmlueck
Copy link
Contributor

gmlueck commented Oct 17, 2024

@KornevNikita now that KhronosGroup/SYCL-CTS#960 is updated, is there some way for you to check that the changes in this PR pass those tests? The Khronos SYCL WG is waiting for us to say that we have an implementation that passes those tests. Once we can say that, I think we can move forward with finalizing the KHR specification.

@KornevNikita
Copy link
Contributor Author

@gmlueck is it enough to say "yes, we can pass them" or we need some result available online?

@gmlueck
Copy link
Contributor

gmlueck commented Oct 17, 2024

@gmlueck is it enough to say "yes, we can pass them" or we need some result available online?

I think the WG will trust us, so I think it is sufficient to say "yes, we pass them". Of course, we really should run the tests to make sure that they do pass.

@KornevNikita
Copy link
Contributor Author

@gmlueck okay, then I'll check that and provide an update.

@KornevNikita
Copy link
Contributor Author

@gmlueck yes, we can pass new tests with this patch.

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