From 758b6a2a2f5785f26503c8411f9cb7cb563c5c20 Mon Sep 17 00:00:00 2001 From: Rafal Rudnicki Date: Wed, 21 Oct 2026 20:18:11 +0000 Subject: [PATCH] Improve SYCL error handling --- sycl/sycl-rs-sys/src/context-sys.rs | 2 +- sycl/sycl-rs-sys/src/device-sys.rs | 8 +- sycl/sycl-rs-sys/src/event-sys.rs | 4 +- sycl/sycl-rs-sys/src/platform-sys.rs | 4 +- sycl/sycl-rs-sys/src/queue-sys.rs | 14 +-- sycl/sycl-rs-sys/src/usm-sys.rs | 2 +- sycl/sycl-rs/benches/batch_memcpy.rs | 6 +- sycl/sycl-rs/examples/kernel_launch.rs | 4 +- sycl/sycl-rs/examples/kernel_launch_derive.rs | 4 +- sycl/sycl-rs/examples/sycl-ls.rs | 7 +- sycl/sycl-rs/src/context.rs | 15 ++-- sycl/sycl-rs/src/device.rs | 23 ++--- sycl/sycl-rs/src/event.rs | 10 ++- sycl/sycl-rs/src/info/event-info.rs | 5 +- sycl/sycl-rs/src/kernel.rs | 4 +- sycl/sycl-rs/src/lib.rs | 14 ++- sycl/sycl-rs/src/platform.rs | 14 +-- sycl/sycl-rs/src/queue.rs | 89 +++++++++++-------- sycl/sycl-rs/src/range.rs | 3 + sycl/sycl-rs/src/usm.rs | 31 ++++--- sycl/sycl-rs/src/usmbox.rs | 15 ++-- sycl/sycl-rs/tests/alloc.rs | 7 +- sycl/sycl-rs/tests/async.rs | 4 +- 23 files changed, 162 insertions(+), 127 deletions(-) diff --git a/sycl/sycl-rs-sys/src/context-sys.rs b/sycl/sycl-rs-sys/src/context-sys.rs index 3e89230..e227c3e 100644 --- a/sycl/sycl-rs-sys/src/context-sys.rs +++ b/sycl/sycl-rs-sys/src/context-sys.rs @@ -20,6 +20,6 @@ pub mod ffi { #[namespace = "sycl_shims"] type Context = crate::types::ffi::Context; - fn new_context(devices: Vec) -> UniquePtr; + fn new_context(devices: Vec) -> Result>; } } diff --git a/sycl/sycl-rs-sys/src/device-sys.rs b/sycl/sycl-rs-sys/src/device-sys.rs index 55c73e4..3d7db24 100644 --- a/sycl/sycl-rs-sys/src/device-sys.rs +++ b/sycl/sycl-rs-sys/src/device-sys.rs @@ -26,8 +26,8 @@ pub mod ffi { #[namespace = "sycl_shims"] type Platform = crate::types::ffi::Platform; - fn get_devices() -> Vec; - fn get_platform(device: &Device) -> UniquePtr; + fn get_devices() -> Result>; + fn get_platform(device: &Device) -> Result>; fn get_device_type(device: &Device) -> DeviceType; fn get_version(device: &Device) -> String; fn get_name(device: &Device) -> String; @@ -39,9 +39,9 @@ pub mod ffi { device: &mut UniquePtr, peer: &Device, value: PeerAccess, - ) -> bool; + ) -> Result; - fn clone(device: &Device) -> UniquePtr; + fn clone(device: &Device) -> Result>; fn has(device: &Device, aspect: Aspect) -> bool; } } diff --git a/sycl/sycl-rs-sys/src/event-sys.rs b/sycl/sycl-rs-sys/src/event-sys.rs index 0d30504..cb3fe8d 100644 --- a/sycl/sycl-rs-sys/src/event-sys.rs +++ b/sycl/sycl-rs-sys/src/event-sys.rs @@ -41,8 +41,8 @@ pub mod ffi { waker: *const SharedWaker, ) -> Result<()>; - fn get_command_execution_status(event: &Event) -> EventCommandStatus; - fn clone(event: &Event) -> UniquePtr; + fn get_command_execution_status(event: &Event) -> Result; + fn clone(event: &Event) -> Result>; } extern "Rust" { diff --git a/sycl/sycl-rs-sys/src/platform-sys.rs b/sycl/sycl-rs-sys/src/platform-sys.rs index 604901b..4383a39 100644 --- a/sycl/sycl-rs-sys/src/platform-sys.rs +++ b/sycl/sycl-rs-sys/src/platform-sys.rs @@ -23,9 +23,9 @@ pub mod ffi { #[namespace = "sycl_shims"] type Platform = crate::types::ffi::Platform; - fn get_platforms() -> Vec; + fn get_platforms() -> Result>; - fn get_devices(platform: &Platform) -> Vec; + fn get_devices(platform: &Platform) -> Result>; fn get_version(platform: &Platform) -> String; fn get_name(platform: &Platform) -> String; diff --git a/sycl/sycl-rs-sys/src/queue-sys.rs b/sycl/sycl-rs-sys/src/queue-sys.rs index 2310585..6e77f37 100644 --- a/sycl/sycl-rs-sys/src/queue-sys.rs +++ b/sycl/sycl-rs-sys/src/queue-sys.rs @@ -33,18 +33,18 @@ pub mod ffi { #[namespace = "sycl_shims"] type Kernel = crate::types::ffi::Kernel; - fn new_queue() -> UniquePtr; - fn new_queue_immediate() -> UniquePtr; - fn new_queue_from_device(device: &Device) -> UniquePtr; + fn new_queue() -> Result>; + fn new_queue_immediate() -> Result>; + fn new_queue_from_device(device: &Device) -> Result>; fn new_queue_from_context_and_device( context: &Context, device: &Device, - ) -> UniquePtr; + ) -> Result>; - fn get_context(queue: &Queue) -> UniquePtr; - fn get_device(queue: &Queue) -> UniquePtr; + fn get_context(queue: &Queue) -> Result>; + fn get_device(queue: &Queue) -> Result>; - fn clone(queue: &Queue) -> UniquePtr; + fn clone(queue: &Queue) -> Result>; /// # Safety /// diff --git a/sycl/sycl-rs-sys/src/usm-sys.rs b/sycl/sycl-rs-sys/src/usm-sys.rs index b727909..ce1bcc4 100644 --- a/sycl/sycl-rs-sys/src/usm-sys.rs +++ b/sycl/sycl-rs-sys/src/usm-sys.rs @@ -48,6 +48,6 @@ pub mod ffi { /// /// `ptr` must be a live USM allocation associated with `queue` and must not be used after /// this call. - unsafe fn free(ptr: *mut u8, queue: &Queue); + unsafe fn free(ptr: *mut u8, queue: &Queue) -> Result<()>; } } diff --git a/sycl/sycl-rs/benches/batch_memcpy.rs b/sycl/sycl-rs/benches/batch_memcpy.rs index ab22e42..467ee3c 100644 --- a/sycl/sycl-rs/benches/batch_memcpy.rs +++ b/sycl/sycl-rs/benches/batch_memcpy.rs @@ -35,13 +35,13 @@ fn memcpy_benchmark(c: &mut Criterion, test_count: u32) -> sycl_rs::Result<()> { let buf_size = used_memory / buf_count; let copy_count = buf_count / 2; - let mut queue = Queue::new(); + let mut queue = Queue::new()?; let mut host_buffers = Vec::>::new(); let mut device_buffers = Vec::>::new(); for _ in 1..copy_count { - let host_buffer = unsafe { queue.alloc_uninit_host::(buf_size) }; - let device_buffer = unsafe { queue.alloc_uninit_device::(buf_size) }; + let host_buffer = unsafe { queue.alloc_uninit_host::(buf_size)? }; + let device_buffer = unsafe { queue.alloc_uninit_device::(buf_size)? }; host_buffers.push(host_buffer); device_buffers.push(device_buffer); diff --git a/sycl/sycl-rs/examples/kernel_launch.rs b/sycl/sycl-rs/examples/kernel_launch.rs index 267b222..375b9dd 100644 --- a/sycl/sycl-rs/examples/kernel_launch.rs +++ b/sycl/sycl-rs/examples/kernel_launch.rs @@ -24,11 +24,11 @@ void iota(sycl::half start, sycl::half *ptr) { #[tokio::main] async fn main() -> sycl_rs::Result<()> { - let mut queue = Queue::new(); + let mut queue = Queue::new()?; let mut device_array = queue.alloc_device::(1024)?.await?; let kernel = queue - .get_context() + .get_context()? .create_kernel_bundle_from_source(IOTA_SRC)? .build()? .get_kernel("iota")?; diff --git a/sycl/sycl-rs/examples/kernel_launch_derive.rs b/sycl/sycl-rs/examples/kernel_launch_derive.rs index 7b72b02..c073773 100644 --- a/sycl/sycl-rs/examples/kernel_launch_derive.rs +++ b/sycl/sycl-rs/examples/kernel_launch_derive.rs @@ -28,11 +28,11 @@ struct IotaArgs<'a> { } fn main() -> sycl_rs::Result<()> { - let mut queue = Queue::new(); + let mut queue = Queue::new()?; let mut array = queue.alloc_shared::(1024)?.wait()?; let kernel = queue - .get_context() + .get_context()? .create_kernel_bundle_from_source(IOTA_SRC)? .build()? .get_kernel("iota")?; diff --git a/sycl/sycl-rs/examples/sycl-ls.rs b/sycl/sycl-rs/examples/sycl-ls.rs index 8a3721e..b43c348 100644 --- a/sycl/sycl-rs/examples/sycl-ls.rs +++ b/sycl/sycl-rs/examples/sycl-ls.rs @@ -8,9 +8,9 @@ use sycl_rs::prelude::*; -fn main() { - for platform in Platform::get_platforms() { - for device in platform.get_devices() { +fn main() -> sycl_rs::Result<()> { + for platform in Platform::get_platforms()? { + for device in platform.get_devices()? { let device_type = device.get_info::(); let platform_name = platform.get_info::(); let device_name = device.get_info::(); @@ -35,4 +35,5 @@ fn main() { ); } } + Ok(()) } diff --git a/sycl/sycl-rs/src/context.rs b/sycl/sycl-rs/src/context.rs index 8ae837c..e340932 100644 --- a/sycl/sycl-rs/src/context.rs +++ b/sycl/sycl-rs/src/context.rs @@ -21,18 +21,21 @@ impl From> for Context { } impl Context { - pub fn new(devices: &[&Device]) -> Self { + /// Creates a context, returning errors from device copies or context creation. + pub fn new(devices: &[&Device]) -> Result { let devices = devices .iter() - .map(|d| DevicePtr { - ptr: (*d).clone().0, + .map(|d| { + Ok(DevicePtr { + ptr: sycl_rs_sys::device::ffi::clone(&d.0)?, + }) }) - .collect::>(); + .collect::>>()?; - ffi::new_context(devices).into() + Ok(ffi::new_context(devices)?.into()) } pub fn create_kernel_bundle_from_source(&self, source: &str) -> Result { - kernel_bundle::ffi::create_kernel_bundle_from_source(&self.0, source).map(Into::into) + Ok(kernel_bundle::ffi::create_kernel_bundle_from_source(&self.0, source)?.into()) } } diff --git a/sycl/sycl-rs/src/device.rs b/sycl/sycl-rs/src/device.rs index d599e55..0789f6d 100644 --- a/sycl/sycl-rs/src/device.rs +++ b/sycl/sycl-rs/src/device.rs @@ -29,17 +29,16 @@ impl From> for Device { impl Device { /// Returns a [`Vec`] containing all the root devices from all SYCL backends /// available in the system which have the device type encapsulated by [`DeviceType`](crate::info::DeviceType). - pub fn get_devices() -> Vec { - ffi::get_devices() + pub fn get_devices() -> Result> { + Ok(ffi::get_devices()? .into_iter() .map(|device| Self(device.ptr)) - .collect() + .collect()) } /// Returns the associated SYCL platform. - pub fn get_platform(&self) -> Platform { - let raw_platform = ffi::get_platform(&self.0); - Platform(raw_platform) + pub fn get_platform(&self) -> Result { + Ok(Platform(ffi::get_platform(&self.0)?)) } /// Returns whether this device has the requested aspect. @@ -56,8 +55,8 @@ impl Device { /// [`PeerAccess::AtomicsSupported`]: When this query returns true, it indicates that this /// device may concurrently access and atomically modify USM device memory allocations located /// on the peer device when peer access is enabled to that device. - pub fn can_access_peer(&mut self, peer: &Device, value: PeerAccess) -> bool { - ffi::can_access_peer(&mut self.0, &peer.0, value) + pub fn can_access_peer(&mut self, peer: &Device, value: PeerAccess) -> Result { + ffi::can_access_peer(&mut self.0, &peer.0, value).map_err(Into::into) } /// Enables this device to access USM device allocations located on the peer device. @@ -67,17 +66,19 @@ impl Device { /// access USM device allocations on the peer device subject to the normal rules about context /// as described in the core SYCL specification. pub fn enable_peer_access(&mut self, peer: &Device) -> Result<()> { - ffi::enable_peer_access(&mut self.0, &peer.0) + ffi::enable_peer_access(&mut self.0, &peer.0).map_err(Into::into) } /// Disables access to the peer device’s memory from this device. pub fn disable_peer_access(&mut self, peer: &Device) -> Result<()> { - ffi::disable_peer_access(&mut self.0, &peer.0) + ffi::disable_peer_access(&mut self.0, &peer.0).map_err(Into::into) } } impl Clone for Device { fn clone(&self) -> Self { - ffi::clone(&self.0).into() + ffi::clone(&self.0) + .expect("failed to clone SYCL device") + .into() } } diff --git a/sycl/sycl-rs/src/event.rs b/sycl/sycl-rs/src/event.rs index 8c094de..003e454 100644 --- a/sycl/sycl-rs/src/event.rs +++ b/sycl/sycl-rs/src/event.rs @@ -26,7 +26,7 @@ impl Event { /// /// Dropping the event does not wait for its completion. pub fn wait(&mut self) -> Result<()> { - ffi::wait(&mut self.0) + ffi::wait(&mut self.0).map_err(Into::into) } } @@ -41,7 +41,9 @@ impl From> for Event { impl Clone for Event { fn clone(&self) -> Self { - ffi::clone(&self.0).into() + ffi::clone(&self.0) + .expect("failed to clone SYCL event") + .into() } } @@ -62,7 +64,7 @@ impl Future for EventFuture { // Set the callback on first Future poll (Futures can't be active until polled) if !*this.set_callback { *this.set_callback = true; - let mut queue = Queue::new_immediate(); + let mut queue = Queue::new_immediate()?; this.shared.waker.register(cx.waker()); // Safety: registered callback will decrement the Arc strong reference count, which is @@ -74,7 +76,7 @@ impl Future for EventFuture { this.queue.replace(queue); } Err(_) => { - return Poll::Ready(result); + return Poll::Ready(result.map_err(Into::into)); } } } else { diff --git a/sycl/sycl-rs/src/info/event-info.rs b/sycl/sycl-rs/src/info/event-info.rs index 8571d9f..30f231e 100644 --- a/sycl/sycl-rs/src/info/event-info.rs +++ b/sycl/sycl-rs/src/info/event-info.rs @@ -6,6 +6,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 // +use crate::Result; use crate::event::Event; use crate::info::Info; use crate::private::Sealed; @@ -15,9 +16,9 @@ use sycl_rs_sys::event::ffi; pub struct CommandExecutionStatus; impl Sealed for CommandExecutionStatus {} impl Info for CommandExecutionStatus { - type Item = crate::info::EventCommandStatus; + type Item = Result; type Target = Event; fn get_item(target: &Self::Target) -> Self::Item { - ffi::get_command_execution_status(&target.0) + ffi::get_command_execution_status(&target.0).map_err(Into::into) } } diff --git a/sycl/sycl-rs/src/kernel.rs b/sycl/sycl-rs/src/kernel.rs index d1ce3a8..490ba74 100644 --- a/sycl/sycl-rs/src/kernel.rs +++ b/sycl/sycl-rs/src/kernel.rs @@ -21,7 +21,7 @@ impl From> for SourceKernelBundle impl SourceKernelBundle { pub fn build(&mut self) -> Result { - ffi::build(&mut self.0).map(Into::into) + Ok(ffi::build(&mut self.0)?.into()) } } @@ -36,7 +36,7 @@ impl From> for ExecutableKern impl ExecutableKernelBundle { pub fn get_kernel(&mut self, name: &str) -> Result { - ffi::get_kernel(&mut self.0, name).map(Into::into) + Ok(ffi::get_kernel(&mut self.0, name)?.into()) } } diff --git a/sycl/sycl-rs/src/lib.rs b/sycl/sycl-rs/src/lib.rs index 0073685..1d9606d 100644 --- a/sycl/sycl-rs/src/lib.rs +++ b/sycl/sycl-rs/src/lib.rs @@ -54,12 +54,12 @@ //! # //! fn main() -> sycl_rs::Result<()> { //! // 1. Create a Queue. It's the main entry point to the SYCL API. -//! let mut queue = Queue::new(); +//! let mut queue = Queue::new()?; //! let mut device_array = queue.alloc_device::(1024)?.wait()?; //! //! // 3. Build a SYCL kernel. //! let kernel = queue -//! .get_context() +//! .get_context()? //! .create_kernel_bundle_from_source(IOTA_SRC)? //! .build()? //! .get_kernel("iota")?; @@ -131,7 +131,15 @@ pub mod range; pub mod usm; pub mod usmbox; -pub type SyclError = cxx::Exception; +/// Errors reported by SYCL operations and USM allocation. +#[derive(Debug, thiserror::Error)] +pub enum SyclError { + #[error(transparent)] + Runtime(#[from] cxx::Exception), + #[error("invalid USM allocation layout")] + InvalidLayout(#[from] std::alloc::LayoutError), +} + pub type Result = std::result::Result; mod private { diff --git a/sycl/sycl-rs/src/platform.rs b/sycl/sycl-rs/src/platform.rs index 74d4ccd..f2ada6c 100644 --- a/sycl/sycl-rs/src/platform.rs +++ b/sycl/sycl-rs/src/platform.rs @@ -8,7 +8,7 @@ use sycl_rs_sys::platform::ffi; -use crate::{device::Device, info::InfoTarget, private::Sealed}; +use crate::{Result, device::Device, info::InfoTarget, private::Sealed}; /// Abstraction for SYCL platform. /// @@ -23,18 +23,18 @@ impl InfoTarget for Platform {} impl Platform { /// Returns a [`Vec`] containing all SYCL platforms from all SYCL backends available in the system. - pub fn get_platforms() -> Vec { - ffi::get_platforms() + pub fn get_platforms() -> Result> { + Ok(ffi::get_platforms()? .into_iter() .map(|platform| Self(platform.ptr)) - .collect() + .collect()) } /// Returns a [`Vec`] containing all the root devices associated with this `Platform`. - pub fn get_devices(&self) -> Vec { - ffi::get_devices(&self.0) + pub fn get_devices(&self) -> Result> { + Ok(ffi::get_devices(&self.0)? .into_iter() .map(|device| Device(device.ptr)) - .collect() + .collect()) } } diff --git a/sycl/sycl-rs/src/queue.rs b/sycl/sycl-rs/src/queue.rs index d0814e4..430d206 100644 --- a/sycl/sycl-rs/src/queue.rs +++ b/sycl/sycl-rs/src/queue.rs @@ -6,7 +6,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 // -use crate::Result; +use crate::{Result, SyclError}; use bytemuck::Pod; use sycl_rs_sys::{queue::ffi, types::ffi::EventPtr}; @@ -32,29 +32,29 @@ pub struct Queue(pub(crate) cxx::UniquePtr); #[allow(clippy::new_without_default)] impl Queue { /// Construct a `Queue` based on the device returned from the default selector. - pub fn new() -> Self { - Self(ffi::new_queue()) + pub fn new() -> Result { + Ok(Self(ffi::new_queue()?)) } /// Construct an immediate `Queue` based on the device returned from the default selector. - pub fn new_immediate() -> Self { - Self(ffi::new_queue_immediate()) + pub fn new_immediate() -> Result { + Ok(Self(ffi::new_queue_immediate()?)) } /// Returns the SYCL queue’s context. - pub fn get_context(&self) -> Context { - ffi::get_context(&self.0).into() + pub fn get_context(&self) -> Result { + Ok(ffi::get_context(&self.0)?.into()) } /// Returns the device the queue is associated with. - pub fn get_device(&self) -> Device { - ffi::get_device(&self.0).into() + pub fn get_device(&self) -> Result { + Ok(ffi::get_device(&self.0)?.into()) } /// Allocates zeroed memory and creates a host-side [`UsmBox`] that can store an array of T. pub fn alloc_host(&mut self, len: usize) -> Result> { unsafe { - let mut array = self.alloc_uninit_host(len); + let mut array = self.alloc_uninit_host(len)?; self.memset(&mut array, 0) .map(|event| EnqueuedUsmBox::new(array, event)) } @@ -63,7 +63,7 @@ impl Queue { /// Allocates zeroed memory and creates a shared [`UsmBox`] that can store an array of T. pub fn alloc_shared(&mut self, len: usize) -> Result> { unsafe { - let mut array = self.alloc_uninit_shared(len); + let mut array = self.alloc_uninit_shared(len)?; self.memset(&mut array, 0) .map(|event| EnqueuedUsmBox::new(array, event)) } @@ -72,7 +72,7 @@ impl Queue { /// Allocates zeroed memory and creates a device [`UsmBox`] that can store an array of T. pub fn alloc_device(&mut self, len: usize) -> Result> { unsafe { - let mut array = self.alloc_uninit_device(len); + let mut array = self.alloc_uninit_device(len)?; self.memset(&mut array, 0) .map(|event| EnqueuedUsmBox::new(array, event)) } @@ -83,8 +83,8 @@ impl Queue { /// # Safety /// /// The array contents are uninitialized and must not be read before they are initialized. - pub unsafe fn alloc_uninit_host(&self, len: usize) -> HostUsmBox { - let allocator = UsmAllocator::from(self); + pub unsafe fn alloc_uninit_host(&self, len: usize) -> Result> { + let allocator = UsmAllocator::try_from(self)?; unsafe { UsmBox::new(allocator, len) } } @@ -93,8 +93,8 @@ impl Queue { /// # Safety /// /// The array contents are uninitialized and must not be read before they are initialized. - pub unsafe fn alloc_uninit_shared(&self, len: usize) -> SharedUsmBox { - let allocator = UsmAllocator::from(self); + pub unsafe fn alloc_uninit_shared(&self, len: usize) -> Result> { + let allocator = UsmAllocator::try_from(self)?; unsafe { UsmBox::new(allocator, len) } } @@ -103,8 +103,8 @@ impl Queue { /// # Safety /// /// The array contents are uninitialized and must not be read before they are initialized. - pub unsafe fn alloc_uninit_device(&self, len: usize) -> DeviceUsmBox { - let allocator = UsmAllocator::from(self); + pub unsafe fn alloc_uninit_device(&self, len: usize) -> Result> { + let allocator = UsmAllocator::try_from(self)?; unsafe { UsmBox::new(allocator, len) } } @@ -136,11 +136,13 @@ impl Queue { let num_bytes = array.get_byte_size(); let dep_events = dep_events .iter() - .map(|e| EventPtr { - ptr: (*e).clone().0, + .map(|e| { + Ok(EventPtr { + ptr: sycl_rs_sys::event::ffi::clone(&e.0)?, + }) }) - .collect::>(); - unsafe { ffi::memset(&mut self.0, ptr, value, num_bytes, dep_events) }.map(Into::into) + .collect::>>()?; + Ok(unsafe { ffi::memset(&mut self.0, ptr, value, num_bytes, dep_events)? }.into()) } /// Submits a barrier to the queue. @@ -152,11 +154,13 @@ impl Queue { pub fn barrier_with_deps(&mut self, dep_events: &[&Event]) -> Result { let dep_events = dep_events .iter() - .map(|e| EventPtr { - ptr: (*e).clone().0, + .map(|e| { + Ok(EventPtr { + ptr: sycl_rs_sys::event::ffi::clone(&e.0)?, + }) }) - .collect::>(); - ffi::barrier(&mut self.0, dep_events).map(Into::into) + .collect::>>()?; + Ok(ffi::barrier(&mut self.0, dep_events)?.into()) } /// Performs a blocking wait for the completion of all enqueued tasks in the queue. Returns an @@ -164,7 +168,7 @@ impl Queue { /// /// Dropping the queue does not wait for its completion. pub fn wait(&mut self) -> Result<()> { - ffi::wait(&mut self.0) + ffi::wait(&mut self.0).map_err(Into::into) } /// Enqueues a kernel object to the queue as an ND-range kernel, using the number of work-items @@ -222,10 +226,12 @@ impl Queue { // TODO: Resolve the C++ lifetime elision issue let dep_events = dep_events .iter() - .map(|e| EventPtr { - ptr: (*e).clone().0, + .map(|e| { + Ok(EventPtr { + ptr: sycl_rs_sys::event::ffi::clone(&e.0)?, + }) }) - .collect::>(); + .collect::>>()?; let num_bytes = src.get_len() * size_of::(); unsafe { @@ -238,20 +244,25 @@ impl Queue { ) } .map(Into::into) + .map_err(Into::into) } } -impl From<&Device> for Queue { - fn from(value: &Device) -> Self { - Self(ffi::new_queue_from_device(&value.0)) +impl TryFrom<&Device> for Queue { + type Error = SyclError; + + fn try_from(value: &Device) -> Result { + Ok(Self(ffi::new_queue_from_device(&value.0)?)) } } -impl From<(&Context, &Device)> for Queue { - fn from(value: (&Context, &Device)) -> Self { - Self(ffi::new_queue_from_context_and_device( +impl TryFrom<(&Context, &Device)> for Queue { + type Error = SyclError; + + fn try_from(value: (&Context, &Device)) -> Result { + Ok(Self(ffi::new_queue_from_context_and_device( &value.0.0, &value.1.0, - )) + )?)) } } @@ -263,6 +274,8 @@ impl From> for Queue { impl Clone for Queue { fn clone(&self) -> Self { - ffi::clone(&self.0).into() + ffi::clone(&self.0) + .expect("failed to clone SYCL queue") + .into() } } diff --git a/sycl/sycl-rs/src/range.rs b/sycl/sycl-rs/src/range.rs index c8b4059..22046cb 100644 --- a/sycl/sycl-rs/src/range.rs +++ b/sycl/sycl-rs/src/range.rs @@ -76,6 +76,7 @@ impl ValidDimension for NdRange<1> { ) } .map(Into::into) + .map_err(Into::into) } } @@ -101,6 +102,7 @@ impl ValidDimension for NdRange<2> { ) } .map(Into::into) + .map_err(Into::into) } } @@ -126,5 +128,6 @@ impl ValidDimension for NdRange<3> { ) } .map(Into::into) + .map_err(Into::into) } } diff --git a/sycl/sycl-rs/src/usm.rs b/sycl/sycl-rs/src/usm.rs index 04f1e69..bdaa1db 100644 --- a/sycl/sycl-rs/src/usm.rs +++ b/sycl/sycl-rs/src/usm.rs @@ -6,12 +6,12 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 // -use crate::queue::Queue; +use crate::{SyclError, queue::Queue}; use allocator_api2::alloc::{AllocError, Allocator}; use sycl_rs_sys::usm::ffi; -use std::{alloc::Layout, marker::PhantomData, ptr::NonNull}; +use std::{alloc::Layout, io::Write, marker::PhantomData, ptr::NonNull}; type CxxResult = cxx::core::result::Result; @@ -48,29 +48,32 @@ pub trait UsmAllocatorKind { /// that allocates memory accessible from the host. pub unsafe trait HostAccessible {} -impl From<&Queue> for UsmAllocator { - fn from(queue: &Queue) -> Self { - Self { - queue: queue.clone(), +impl TryFrom<&Queue> for UsmAllocator { + type Error = SyclError; + + fn try_from(queue: &Queue) -> crate::Result { + Ok(Self { + queue: sycl_rs_sys::queue::ffi::clone(&queue.0)?.into(), _kind: PhantomData, - } + }) } } unsafe impl Allocator for UsmAllocator { fn allocate(&self, layout: Layout) -> Result, AllocError> { let ptr = unsafe { T::alloc(layout.align(), layout.size(), &self.queue) } - .map_err(|_e| AllocError)?; - + .map_err(|_| AllocError)?; let ptr = NonNull::new(ptr).ok_or(AllocError)?; - let slice = NonNull::slice_from_raw_parts(ptr, layout.size()); - - Ok(slice) + Ok(NonNull::slice_from_raw_parts(ptr, layout.size())) } unsafe fn deallocate(&self, ptr: NonNull, _layout: Layout) { - unsafe { - ffi::free(ptr.as_ptr(), &self.queue.0); + if let Err(error) = unsafe { ffi::free(ptr.as_ptr(), &self.queue.0) } { + // A failed stderr write must not cause a second panic during unwinding. + let _ = writeln!( + std::io::stderr().lock(), + "sycl-rs: USM deallocation failed: {error}" + ); } } } diff --git a/sycl/sycl-rs/src/usmbox.rs b/sycl/sycl-rs/src/usmbox.rs index b9ba1ab..6150d48 100644 --- a/sycl/sycl-rs/src/usmbox.rs +++ b/sycl/sycl-rs/src/usmbox.rs @@ -48,20 +48,21 @@ pub struct UsmBox { impl UsmBox { /// Creates a new array given an allocator. + /// Allocation failures invoke [`handle_alloc_error`]; invalid layouts return an error. /// Safety: returns uninitialized memory. - pub(crate) unsafe fn new(allocator: A, len: usize) -> Self { - let layout = Layout::array::(len).unwrap(); + pub(crate) unsafe fn new(allocator: A, len: usize) -> Result { + let layout = Layout::array::(len)?; let ptr = match allocator.allocate(layout) { Ok(ptr) => ptr, - _ => handle_alloc_error(layout), + Err(_) => handle_alloc_error(layout), }; - Self { + Ok(Self { data: ptr.cast(), len, layout, allocator, - } + }) } pub(crate) fn get_byte_ptr(&self) -> *mut u8 { @@ -98,9 +99,7 @@ impl DerefMut for UsmBox { impl Drop for UsmBox { fn drop(&mut self) { - unsafe { - self.allocator.deallocate(self.data.cast(), self.layout); - } + unsafe { self.allocator.deallocate(self.data.cast(), self.layout) }; } } diff --git a/sycl/sycl-rs/tests/alloc.rs b/sycl/sycl-rs/tests/alloc.rs index 800b9a1..fd3a20b 100644 --- a/sycl/sycl-rs/tests/alloc.rs +++ b/sycl/sycl-rs/tests/alloc.rs @@ -9,13 +9,14 @@ use sycl_rs::queue::Queue; #[test] -fn shared_allocation_host_values() { - let queue = Queue::new(); - let mut array = unsafe { queue.alloc_uninit_shared::(10) }; +fn shared_allocation_host_values() -> sycl_rs::Result<()> { + let queue = Queue::new()?; + let mut array = unsafe { queue.alloc_uninit_shared::(10)? }; for (index, value) in array.iter_mut().enumerate() { *value = index as u32; } assert_eq!(array.as_ref(), &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]); + Ok(()) } diff --git a/sycl/sycl-rs/tests/async.rs b/sycl/sycl-rs/tests/async.rs index 599cfe3..f6732a2 100644 --- a/sycl/sycl-rs/tests/async.rs +++ b/sycl/sycl-rs/tests/async.rs @@ -10,11 +10,11 @@ use sycl_rs::queue::Queue; #[tokio::test] async fn check_for_select_support() -> sycl_rs::Result<()> { - let mut queue = Queue::new(); + let mut queue = Queue::new()?; let _selected_array = tokio::select! { array1 = queue.alloc_device::(1024)? => array1, array2 = queue.alloc_device::(10240)? => array2 - }; + }?; Ok(()) }