[SYCL] Deprecate info::*::reference_count#22521
Conversation
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. |
| ur_result_t result = UR_RESULT_SUCCESS; | ||
| getAdapter().call<UrApiKind::urQueueGetInfo>( | ||
| MQueue, UR_QUEUE_INFO_REFERENCE_COUNT, sizeof(result), &result, nullptr); | ||
| return result; |
| 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; | ||
| }; |
| 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; | ||
| }; |
|
@crystarm please fix pre-commit issues |
KornevNikita
left a comment
KornevNikita
left a comment
There was a problem hiding this comment.
LGTM, @uditagarwal97 could you please take one more look
|
Hi @crystarm, The PR is good to be merged but there's one thing blocking it.
Please check out https://llvm.org/docs/DeveloperPolicy.html#email-addresses - you'd have to change your Github account setting to allow committing from your personal email address. We don't allow committing from anonymous email addresses (like from noreply.github.com). Just changing your Github account settings should suffice - you don't have update the commits to include your personal email. |
crystarm
force-pushed
the
deprecate-info-reference-count
branch
from
a7a5aa4 to
ee1ae4f
Compare
|
@uditagarwal97 I also disabled the "Keep my email addresses private" option in my GitHub settings. |
okay, no worries. |
|
Thanks for working on this @crystarm 🚀 |
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.