[SYCL] Deprecate info::*::reference_count#22521
Draft
crystarm wants to merge 3 commits into
Draft
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR deprecates the legacy SYCL 1.2.1 info::*::reference_count descriptors (not part of SYCL 2020) across the context, device, event, kernel, and queue info namespaces, and gates their availability behind __INTEL_PREVIEW_BREAKING_CHANGES for future removal during an ABI-break window.
Changes:
- Adds
__SYCL_DEPRECATEDannotations toinfo::{context,device,event,kernel,queue}::reference_count. - Wraps the corresponding explicit template instantiations and
get_infospecializations in#ifndef __INTEL_PREVIEW_BREAKING_CHANGES. - Ensures the public info descriptors themselves are also conditionally available under the same macro.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sycl/source/queue.cpp | Gates queue::get_info<info::queue::reference_count>() explicit instantiation behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/kernel.cpp | Gates kernel::get_info_impl<info::kernel::reference_count>() explicit instantiation behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/event.cpp | Gates event::get_info<info::event::reference_count>() explicit instantiation behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/device.cpp | Gates device::get_info_impl<info::device::reference_count>() explicit instantiation behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/context.cpp | Gates context::get_info<info::context::reference_count>() explicit instantiation behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/detail/queue_impl.cpp | Gates the queue_impl reference_count query specialization behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/detail/event_impl.cpp | Gates the event_impl reference_count query specialization behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/source/detail/context_impl.cpp | Gates the context_impl reference_count query specialization behind __INTEL_PREVIEW_BREAKING_CHANGES. |
| sycl/include/sycl/info/queue.hpp | Deprecates and gates info::queue::reference_count. |
| sycl/include/sycl/info/kernel.hpp | Deprecates and gates info::kernel::reference_count. |
| sycl/include/sycl/info/event.hpp | Deprecates and gates info::event::reference_count. |
| sycl/include/sycl/info/device.hpp | Deprecates and gates info::device::reference_count. |
| sycl/include/sycl/info/context.hpp | Deprecates and gates info::context::reference_count. |
Comment on lines
68
to
71
| ur_result_t result = UR_RESULT_SUCCESS; | ||
| getAdapter().call<UrApiKind::urQueueGetInfo>( | ||
| MQueue, UR_QUEUE_INFO_REFERENCE_COUNT, sizeof(result), &result, nullptr); | ||
| return result; |
Comment on lines
+42
to
46
| struct __SYCL_DEPRECATED("info::kernel::reference_count is not part of " | ||
| "SYCL 2020") reference_count | ||
| : kernel_traits<UR_KERNEL_INFO_REFERENCE_COUNT> { | ||
| using return_type = uint32_t; | ||
| }; |
Comment on lines
+40
to
44
| struct __SYCL_DEPRECATED("info::event::reference_count is not part of " | ||
| "SYCL 2020") reference_count | ||
| : event_traits<UR_EVENT_INFO_REFERENCE_COUNT> { | ||
| using return_type = uint32_t; | ||
| }; |
Contributor
|
@crystarm please fix pre-commit issues |
KornevNikita
approved these changes
Jul 7, 2026
KornevNikita
left a comment
KornevNikita
left a comment
Contributor
There was a problem hiding this comment.
LGTM, @uditagarwal97 could you please take one more look
|
|
||
| #pragma once | ||
|
|
||
| #include <sycl/detail/defines_elementary.hpp> |
Contributor
There was a problem hiding this comment.
Let's guard this include here and in other files so that we don't forget to remove it.
| print_info<info::device::partition_type_affinity_domain, | ||
| info::partition_affinity_domain>(dev, | ||
| "Partition type affinity domain"); | ||
| print_info<info::device::reference_count, sycl::opencl::cl_uint>( |
Contributor
There was a problem hiding this comment.
Tests also can be guarded with #ifdef __INTEL_PREVIEW_BREAKING_CHANGES, although I'm okay with just removing this, up to @uditagarwal97
Contributor
There was a problem hiding this comment.
I'm also okay with just removing this.
| @@ -0,0 +1,47 @@ | |||
| // UNSUPPORTED: preview-mode | |||
| // UNSUPPORTED-INTENDED: reference_count is unavailable in preview mode. | |||
| // RUN: %{build} -o %t.out | |||
Contributor
There was a problem hiding this comment.
Would -fsyntax-only suffice here? since we are not running the test anyway.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22504.
Summary
info::reference_countdescriptors exist in five info namespaces (context,device,event,kernel,queue).This PR deprecates all five descriptors and guards them behind
__INTEL_PREVIEW_BREAKING_CHANGES.Notes
Three e2e tests (
Basic/kernel_info.cpp,Basic/info.cpp,Regression/dummy_event_info.cpp) still referencereference_countand will emit deprecation warnings. I guess they should be cleaned up in a follow-up.