From 7b26a8e623b4e4224fc51e2e2a4a5188e2c68dc6 Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Tue, 15 Jul 2025 11:17:11 -0700 Subject: [PATCH] [NFC][SYCL] Change `context_impl::getDevices` to return `devices_range` `devices_range` helper was added in https://github.com/intel/llvm/pull/19405 to facilitate ongoing refactoring of using more raw ptr/ref to `*_impl` objects. Also update some of the callsites that can use that instead of more expensive `get_info()`. --- sycl/source/detail/context_impl.hpp | 4 +- .../source/detail/device_global_map_entry.cpp | 7 ++-- sycl/source/detail/helpers.hpp | 4 ++ sycl/source/detail/image_impl.cpp | 42 ++++++++----------- sycl/source/detail/image_impl.hpp | 3 +- .../program_manager/program_manager.cpp | 6 +-- .../source/detail/scheduler/graph_builder.cpp | 9 ++-- sycl/source/detail/usm/usm_impl.cpp | 4 +- 8 files changed, 36 insertions(+), 43 deletions(-) diff --git a/sycl/source/detail/context_impl.hpp b/sycl/source/detail/context_impl.hpp index 4be6ee9ac60f8..bf3b326c6f699 100644 --- a/sycl/source/detail/context_impl.hpp +++ b/sycl/source/detail/context_impl.hpp @@ -130,9 +130,7 @@ class context_impl : public std::enable_shared_from_this { /// \return an instance of raw UR context handle. const ur_context_handle_t &getHandleRef() const; - /// Unlike `get_info', this function returns a - /// reference. - const std::vector &getDevices() const { return MDevices; } + devices_range getDevices() const { return MDevices; } using CachedLibProgramsT = std::map, diff --git a/sycl/source/detail/device_global_map_entry.cpp b/sycl/source/detail/device_global_map_entry.cpp index d79715e52c1e2..2bad4a896958a 100644 --- a/sycl/source/detail/device_global_map_entry.cpp +++ b/sycl/source/detail/device_global_map_entry.cpp @@ -104,7 +104,7 @@ DeviceGlobalMapEntry::getOrAllocateDeviceGlobalUSM(const context &Context) { "USM allocations should not be acquired for device_global with " "device_image_scope property."); context_impl &CtxImpl = *getSyclObjImpl(Context); - device_impl &DevImpl = *getSyclObjImpl(CtxImpl.getDevices().front()); + device_impl &DevImpl = CtxImpl.getDevices().front(); std::lock_guard Lock(MDeviceToUSMPtrMapMutex); auto DGUSMPtr = MDeviceToUSMPtrMap.find({&DevImpl, &CtxImpl}); @@ -153,9 +153,8 @@ DeviceGlobalMapEntry::getOrAllocateDeviceGlobalUSM(const context &Context) { void DeviceGlobalMapEntry::removeAssociatedResources( const context_impl *CtxImpl) { std::lock_guard Lock{MDeviceToUSMPtrMapMutex}; - for (device Device : CtxImpl->getDevices()) { - auto USMPtrIt = - MDeviceToUSMPtrMap.find({getSyclObjImpl(Device).get(), CtxImpl}); + for (device_impl &Device : CtxImpl->getDevices()) { + auto USMPtrIt = MDeviceToUSMPtrMap.find({&Device, CtxImpl}); if (USMPtrIt != MDeviceToUSMPtrMap.end()) { DeviceGlobalUSMMem &USMMem = USMPtrIt->second; detail::usm::freeInternal(USMMem.MPtr, CtxImpl); diff --git a/sycl/source/detail/helpers.hpp b/sycl/source/detail/helpers.hpp index d79440a912677..1a362023dec15 100644 --- a/sycl/source/detail/helpers.hpp +++ b/sycl/source/detail/helpers.hpp @@ -102,6 +102,10 @@ template class iterator_range { iterator_range(IterTy Begin, IterTy End, size_t Size) : Begin(Begin), End(End), Size(Size) {} + iterator_range() + : iterator_range(static_cast(nullptr), + static_cast(nullptr), 0) {} + template iterator_range(const ContainerTy &Container) : iterator_range(Container.begin(), Container.end(), Container.size()) {} diff --git a/sycl/source/detail/image_impl.cpp b/sycl/source/detail/image_impl.cpp index 43568f7dfe6c0..5affc3116a30d 100644 --- a/sycl/source/detail/image_impl.cpp +++ b/sycl/source/detail/image_impl.cpp @@ -23,12 +23,11 @@ uint8_t GImageStreamID; #endif template -static bool checkImageValueRange(const std::vector &Devices, - const size_t Value) { - return Value >= 1 && std::all_of(Devices.cbegin(), Devices.cend(), - [Value](const device &Dev) { - return Value <= Dev.get_info(); - }); +static bool checkImageValueRange(devices_range Devices, const size_t Value) { + return Value >= 1 && + std::all_of(Devices.begin(), Devices.end(), [Value](device_impl &Dev) { + return Value <= Dev.get_info(); + }); } template static bool checkAnyImpl(T) { @@ -345,46 +344,47 @@ void *image_impl::allocateMem(context_impl *Context, bool InitFromUserData, bool image_impl::checkImageDesc(const ur_image_desc_t &Desc, context_impl *Context, void *UserPtr) { + devices_range Devices = Context ? Context->getDevices() : devices_range{}; if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE1D, UR_MEM_TYPE_IMAGE1D_ARRAY, UR_MEM_TYPE_IMAGE2D_ARRAY, UR_MEM_TYPE_IMAGE2D) && - !checkImageValueRange( - getDevices(Context), Desc.width)) + !checkImageValueRange(Devices, + Desc.width)) throw exception(make_error_code(errc::invalid), "For a 1D/2D image/image array, the width must be a Value " ">= 1 and <= info::device::image2d_max_width"); if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE3D) && - !checkImageValueRange( - getDevices(Context), Desc.width)) + !checkImageValueRange(Devices, + Desc.width)) throw exception(make_error_code(errc::invalid), "For a 3D image, the width must be a Value >= 1 and <= " "info::device::image3d_max_width"); if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE2D, UR_MEM_TYPE_IMAGE2D_ARRAY) && - !checkImageValueRange( - getDevices(Context), Desc.height)) + !checkImageValueRange(Devices, + Desc.height)) throw exception(make_error_code(errc::invalid), "For a 2D image or image array, the height must be a Value " ">= 1 and <= info::device::image2d_max_height"); if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE3D) && - !checkImageValueRange( - getDevices(Context), Desc.height)) + !checkImageValueRange(Devices, + Desc.height)) throw exception(make_error_code(errc::invalid), "For a 3D image, the heightmust be a Value >= 1 and <= " "info::device::image3d_max_height"); if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE3D) && - !checkImageValueRange( - getDevices(Context), Desc.depth)) + !checkImageValueRange(Devices, + Desc.depth)) throw exception(make_error_code(errc::invalid), "For a 3D image, the depth must be a Value >= 1 and <= " "info::device::image2d_max_depth"); if (checkAny(Desc.type, UR_MEM_TYPE_IMAGE1D_ARRAY, UR_MEM_TYPE_IMAGE2D_ARRAY) && - !checkImageValueRange( - getDevices(Context), Desc.arraySize)) + !checkImageValueRange(Devices, + Desc.arraySize)) throw exception(make_error_code(errc::invalid), "For a 1D and 2D image array, the array_size must be a " "Value >= 1 and <= info::device::image_max_array_size."); @@ -451,12 +451,6 @@ bool image_impl::checkImageFormat(const ur_image_format_t &Format, return true; } -std::vector image_impl::getDevices(context_impl *Context) { - if (!Context) - return {}; - return Context->get_info(); -} - void image_impl::sampledImageConstructorNotification( const detail::code_location &CodeLoc, void *UserObj, const void *HostObj, uint32_t Dim, size_t Range[3], image_format Format, diff --git a/sycl/source/detail/image_impl.hpp b/sycl/source/detail/image_impl.hpp index 0d8a2b50a499b..214a7a79f9404 100644 --- a/sycl/source/detail/image_impl.hpp +++ b/sycl/source/detail/image_impl.hpp @@ -35,6 +35,7 @@ class accessor; class handler; namespace detail { +class devices_range; // utility functions and typedefs for image_impl using image_allocator = aligned_allocator; @@ -297,8 +298,6 @@ class image_impl final : public SYCLMemObjT { void unsampledImageDestructorNotification(void *UserObj); private: - std::vector getDevices(context_impl *Context); - ur_mem_type_t getImageType() { if (MDimensions == 1) return (MIsArrayImage ? UR_MEM_TYPE_IMAGE1D_ARRAY : UR_MEM_TYPE_IMAGE1D); diff --git a/sycl/source/detail/program_manager/program_manager.cpp b/sycl/source/detail/program_manager/program_manager.cpp index 059c170df4bb1..9177bf6a5956c 100644 --- a/sycl/source/detail/program_manager/program_manager.cpp +++ b/sycl/source/detail/program_manager/program_manager.cpp @@ -124,10 +124,10 @@ static bool isDeviceBinaryTypeSupported(context_impl &ContextImpl, if (ContextBackend == backend::ext_oneapi_cuda) return false; - const std::vector &Devices = ContextImpl.getDevices(); + devices_range Devices = ContextImpl.getDevices(); // Program type is SPIR-V, so we need a device compiler to do JIT. - for (const device &D : Devices) { + for (device_impl &D : Devices) { if (!D.get_info()) return false; } @@ -143,7 +143,7 @@ static bool isDeviceBinaryTypeSupported(context_impl &ContextImpl, return true; } - for (const device &D : Devices) { + for (device_impl &D : Devices) { // We need cl_khr_il_program extension to be present // and we can call clCreateProgramWithILKHR using the extension std::vector Extensions = diff --git a/sycl/source/detail/scheduler/graph_builder.cpp b/sycl/source/detail/scheduler/graph_builder.cpp index 9257c6b5e31bf..3a2a8c8b46381 100644 --- a/sycl/source/detail/scheduler/graph_builder.cpp +++ b/sycl/source/detail/scheduler/graph_builder.cpp @@ -213,10 +213,9 @@ Scheduler::GraphBuilder::getOrInsertMemObjRecord(queue_impl *Queue, // which means that there is already an allocation(cl_mem) in some context. // Registering this allocation in the SYCL graph. - std::vector Devices = - InteropCtxPtr->get_info(); - assert(Devices.size() != 0); - device_impl &Dev = *detail::getSyclObjImpl(Devices[0]); + devices_range Devices = InteropCtxPtr->getDevices(); + assert(!Devices.empty()); + device_impl &Dev = Devices.front(); // Since all the Scheduler commands require queue but we have only context // here, we need to create a dummy queue bound to the context and one of the @@ -675,7 +674,7 @@ static bool checkHostUnifiedMemory(context_impl *Ctx) { if (Ctx == nullptr) return true; - for (const device &Device : Ctx->getDevices()) { + for (device_impl &Device : Ctx->getDevices()) { if (!Device.get_info()) return false; } diff --git a/sycl/source/detail/usm/usm_impl.cpp b/sycl/source/detail/usm/usm_impl.cpp index 4e6e9750c3484..2f6d6fd5688f8 100644 --- a/sycl/source/detail/usm/usm_impl.cpp +++ b/sycl/source/detail/usm/usm_impl.cpp @@ -581,13 +581,13 @@ device get_pointer_device(const void *Ptr, const context &Ctxt) { // Check if ptr is a host allocation if (get_pointer_type(Ptr, Ctxt) == alloc::host) { - auto Devs = detail::getSyclObjImpl(Ctxt)->getDevices(); + detail::devices_range Devs = detail::getSyclObjImpl(Ctxt)->getDevices(); if (Devs.size() == 0) throw exception(make_error_code(errc::invalid), "No devices in passed context!"); // Just return the first device in the context - return Devs[0]; + return detail::createSyclObjFromImpl(Devs.front()); } ur_device_handle_t DeviceId;