Add memcpy binding - #11
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR expands the oneapi-rs USM buffer abstractions by adding device-allocated buffers and a generic Queue::copy (memcpy) binding, and updates the kernel launch example to demonstrate device→host transfers using the async event model.
Changes:
- Add a
DeviceAllocatorand aHostAccessiblemarker trait to differentiate host-accessible vs device-only USM buffers. - Add
Queue::alloc_device*andQueue::{copy, copy_with_deps}backed by a new SYCLqueue::memcpyFFI binding. - Update
examples/kernel_launch.rsto allocate device buffers, launch a kernel, then copy results back to host and print.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| oneapi-rs/src/usm.rs | Introduces host-accessibility marker trait and exposes a public device allocator kind. |
| oneapi-rs/src/queue.rs | Adds device allocation helpers and new copy/memcpy APIs on Queue. |
| oneapi-rs/src/buffer.rs | Restricts slice deref to host-accessible allocators and adds buffer length accessor. |
| oneapi-rs/examples/kernel_launch.rs | Updates example to use async/await and device→host copy. |
| oneapi-rs-sys/src/queue.cpp | Adds C++ shim for queue->memcpy(...) with dependency events. |
| oneapi-rs-sys/src/queue-sys.rs | Exposes memcpy over cxx bridge to Rust. |
| oneapi-rs-sys/include/queue.hpp | Declares the new memcpy shim in the sys header. |
Comments suppressed due to low confidence (1)
oneapi-rs/src/queue.rs:201
- Same as
copy: please constrainTtoPodfor this raw memcpy-based API so the safe surface area stays bitwise-sound.
pub fn copy_with_deps<T, A1, A2>(
&mut self,
src: &Buffer<T, A1>,
dst: &mut Buffer<T, A2>,
dep_events: &[&Event],
) -> Event
where
A1: UsmAlloc,
A2: UsmAlloc,
{
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
oneapi-rs/src/queue.rs:214
size_of::<T>()is not in scope in this module, which will cause a compile error. Use a qualified path (or import it explicitly).
let amount = min(src.get_len(), dst.get_len());
let num_bytes = amount * size_of::<T>();
unsafe {
oneapi-rs/src/usm.rs:34
HostAccessibleis declared as anunsafe trait, but it doesn't document the safety contract that implementers must uphold. For unsafe marker traits, add a# Safetysection that spells out the required invariant (e.g., that allocations are directly readable/writable from the host for the lifetime of the allocation).
/// A marker trait for host-accessible USM allocators.
pub unsafe trait HostAccessible {}
bratpiorka
approved these changes
Jul 28, 2026
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.
This PR adds: