[SYCL][NFC] Consolidate all uses of OpenCL types, functions#22532
Conversation
|
Note to reviewers: In the follow-up PR, I will entirely remove inclusion of OpenCL headers when compiling for SYCL Device, that requires many changes in E2E tests, that's why will do it in a separate PR. |
77be2cc to
a53f32c
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Refactors SYCL OpenCL interop code to route all OpenCL native types through new aliases in sycl/detail/cl.h, enabling builds (especially device/JIT) to avoid pulling in OpenCL host headers while keeping host behavior intact.
Changes:
- Introduced
OpenCL*Ttype aliases (e.g.,OpenCLContextT,OpenCLMemT) and centralized OpenCL retain helpers insycl/detail/cl.h. - Mechanically updated public/internal APIs and implementations to use the new aliases instead of raw
cl_*types. - Updated various docs/comments and added a test include related to OpenCL types.
Reviewed changes
Copilot reviewed 45 out of 45 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| sycl/test/basic_tests/types.cpp | Adds an OpenCL header include to exercise/validate type compatibility in tests. |
| sycl/source/sampler.cpp | Updates interop constructor signature to use OpenCLSamplerT. |
| sycl/source/queue.cpp | Updates interop constructor / get() return type and adjusts related comment. |
| sycl/source/platform.cpp | Updates interop constructor / get() return type to OpenCLPlatformT. |
| sycl/source/kernel.cpp | Updates interop constructor / get() return type and uses centralized retain helper. |
| sycl/source/image.cpp | Updates interop constructor parameter type to OpenCLMemT. |
| sycl/source/event.cpp | Updates interop constructor parameter type and uses centralized retain helper. |
| sycl/source/device.cpp | Updates interop constructor / get() return type and uses centralized retain helper. |
| sycl/source/detail/sycl_mem_obj_t.hpp | Updates interop ctor parameter type to OpenCLMemT. |
| sycl/source/detail/sycl_mem_obj_t.cpp | Updates exception text and replaces clRetainMemObject calls with helper. |
| sycl/source/detail/sycl_mem_obj_i.hpp | Updates comment references from cl_mem to OpenCLMemT. |
| sycl/source/detail/scheduler/scheduler.hpp | Updates documentation example to reference OpenCLEventT. |
| sycl/source/detail/scheduler/graph_builder.cpp | Updates comments to reference OpenCLMemT. |
| sycl/source/detail/sampler_impl.hpp | Updates ctor parameter type to OpenCLSamplerT. |
| sycl/source/detail/sampler_impl.cpp | Updates ctor signature formatting/types to OpenCLSamplerT. |
| sycl/source/detail/queue_impl.hpp | Updates get() return type and replaces retain call with helper. |
| sycl/source/detail/queue_impl.cpp | Replaces clRetainCommandQueue with centralized retain helper. |
| sycl/source/detail/platform_impl.hpp | Updates get() return type/casts to OpenCLPlatformT. |
| sycl/source/detail/memory_manager.hpp | Updates comments to reference OpenCLMemT. |
| sycl/source/detail/memory_manager.cpp | Updates comments to reference OpenCLMemT. |
| sycl/source/detail/kernel_impl.hpp | Updates docs/return types and replaces retain calls with helper. |
| sycl/source/detail/kernel_impl.cpp | Updates exception text and documentation example type name. |
| sycl/source/detail/image_impl.hpp | Updates interop ctor parameter type to OpenCLMemT. |
| sycl/source/detail/image_impl.cpp | Updates interop ctor parameter type to OpenCLMemT. |
| sycl/source/detail/event_impl.cpp | Replaces clRetainEvent with centralized retain helper. |
| sycl/source/detail/device_impl.hpp | Updates get() return type/docs to OpenCLDeviceIdT. |
| sycl/source/detail/device_impl.cpp | Updates get() return type/cast and replaces retain calls with helper. |
| sycl/source/detail/device_image_impl.hpp | Replaces clRetainProgram with centralized retain helper. |
| sycl/source/detail/context_impl.hpp | Updates get() return type/docs to OpenCLContextT. |
| sycl/source/detail/context_impl.cpp | Updates get() return type/cast and replaces retain calls with helper. |
| sycl/source/detail/buffer_impl.hpp | Updates interop ctor parameter type to OpenCLMemT. |
| sycl/source/detail/buffer_impl.cpp | Replaces clRetainMemObject with centralized retain helper. |
| sycl/source/context.cpp | Updates interop ctor / get() return type to OpenCLContextT. |
| sycl/source/backend.cpp | Replaces OpenCL retain calls with centralized retain helpers. |
| sycl/include/sycl/sampler.hpp | Updates interop ctor signature to OpenCLSamplerT. |
| sycl/include/sycl/queue.hpp | Updates interop ctor / get() return type and docs to OpenCLCommandQueueT. |
| sycl/include/sycl/platform.hpp | Updates interop ctor / get() return type and docs to OpenCLPlatformT. |
| sycl/include/sycl/kernel.hpp | Updates interop ctor / get() return type and docs to OpenCLKernelT. |
| sycl/include/sycl/image.hpp | Updates interop ctors parameter type to OpenCLMemT. |
| sycl/include/sycl/handler.hpp | Updates type-traits logic to treat OpenCLMemT as the interop argument type. |
| sycl/include/sycl/event.hpp | Updates interop ctor parameter type and docs to OpenCLEventT. |
| sycl/include/sycl/device.hpp | Updates interop ctor / get() return type and docs to OpenCLDeviceIdT. |
| sycl/include/sycl/detail/cl.h | Introduces OpenCL*T aliases and centralized retain helpers; adjusts OpenCL header inclusion behavior. |
| sycl/include/sycl/detail/backend_traits_opencl.hpp | Switches backend traits and casts from raw cl_* types to OpenCL*T aliases. |
| sycl/include/sycl/context.hpp | Updates interop ctor / get() return type and docs to OpenCLContextT. |
Comments suppressed due to low confidence (4)
sycl/test/basic_tests/types.cpp:1
- Adding a direct OpenCL header include to a basic SYCL test increases the test suite’s environmental requirements (OpenCL headers must be present) and undermines the consolidation goal of avoiding OpenCL header inclusion where possible. Prefer including
sycl/detail/cl.h(or another SYCL-provided header) and validating the aliasing viaOpenCL*Ttypes instead, so the test doesn’t depend on system OpenCL headers.
sycl/source/detail/sycl_mem_obj_i.hpp:1 - Correct typo in comment: change “obect” to “object”.
sycl/source/detail/scheduler/scheduler.hpp:1 - The line break in this documentation example makes
Context2look like a standalone code line inside the snippet, which is misleading. Keep the comment on a single line (e.g.,// Scope of Context2) or ensure the continuation line is also a comment so the example remains valid/readable.
sycl/source/detail/sycl_mem_obj_t.cpp:1 - This exception message now exposes the internal alias name
OpenCLMemT, which may be confusing to users (it’s not an OpenCL-defined type name and may not appear in user code). Consider wording it as “context of the OpenCL memory object (cl_mem)” or similar user-facing terminology, while keeping the internal type aliasing purely in code.
|
PR to remove OpenCL headers when compiling for device: #22533 |
bader
left a comment
There was a problem hiding this comment.
Refactoring needed to remove inclusion of OpenCL headers when compiling for SYCL device.
OpenCL headers must be removed from the SYCL in general. SYCL-2020 implementation is supposed to use unified runtime types/functions. OpenCL types/functions are supposed to be used only for SYCL interoperability with OpenCL API.
|
CI failure is unrelated. |
Refactoring needed to remove inclusion of OpenCL headers when compiling for SYCL device.
This PR does mechanical renaming of every OpenCL interop host type to a new alias in
detail/cl.h—cl_context→OpenCLContextT,cl_mem→OpenCLMemT, etc. — across headers and sources.This alias resolves to the corresponding OpenCL type, except when JIT compiling for SYCL device.