Skip to content

Conversation

@aarongreig
Copy link
Contributor

@aarongreig aarongreig commented Aug 9, 2024

It currently takes a platform handle, which is problematic for the sycl RT because its make_device api only takes a native handle, so to figure out the correct platform handle to pass at best we'd need to do some backend specific querying of the native object, but even then that isn't always possible as not all backends have a platform equivalent.

The platform handle does currently enable an optional (slightly) faster path to return the correct device in some adapter implementations but this isn't essential for them to work, so really its primary purpose is to serve as the wrapped UR handle for the loader to work. This purpose is equally well served by an adapter handle, which will also be a lot easier for the sycl rt to correctly provide.

@github-actions github-actions bot added loader Loader related feature/bug conformance Conformance test suite issues. specification Changes or additions to the specification level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues native-cpu Native CPU adapter specific issues labels Aug 9, 2024
@aarongreig aarongreig marked this pull request as ready for review August 9, 2024 09:58
@aarongreig aarongreig requested review from a team as code owners August 9, 2024 09:58
@aarongreig aarongreig requested a review from keyradical August 9, 2024 09:58
It currently takes a platform handle, which is problematic for the sycl
RT because its make_device api only takes a native handle, so to figure
out the correct platform handle to pass at best we'd need to do some
backend specific querying of the native object, but even then that isn't
always possible as not all backends have a platform equivalent.

The platform handle does currently enable an optional (slightly) faster
path to return the correct device in some adapter implementations but
this isn't essential for them to work, so really its primary purpose is
to serve as the wrapped UR handle for the loader to work. This purpose
is equally well served by an adapter handle, which will also be a lot
easier for the sycl rt to correctly provide.
@aarongreig aarongreig force-pushed the aaron/changeDeviceCreateWithNativeParam branch from f942502 to a4c6e91 Compare August 9, 2024 10:05
@aarongreig
Copy link
Contributor Author

LLVM PR intel/llvm#15023 sycl-e2e jobs here will fail due to missing accompanying rt change

@aarongreig aarongreig added the v0.10.x Include in the v0.10.x release label Aug 13, 2024
@aarongreig
Copy link
Contributor Author

ping @oneapi-src/unified-runtime-native-cpu-write @oneapi-src/unified-runtime-cuda-write @oneapi-src/unified-runtime-hip-write @oneapi-src/unified-runtime-level-zero-write @oneapi-src/unified-runtime-maintain this is necessary a fix for the release

@kbenzie kbenzie added the ready to merge Added to PR's which are ready to merge label Aug 14, 2024
@omarahmed1111 omarahmed1111 merged commit 0342c95 into oneapi-src:main Aug 15, 2024
kbenzie pushed a commit that referenced this pull request Aug 20, 2024
…NativeParam

Change urDeviceCreateWithNativeHandle to take an adapter handle.
@kbenzie kbenzie mentioned this pull request Aug 20, 2024
53 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

conformance Conformance test suite issues. cuda CUDA adapter specific issues hip HIP adapter specific issues level-zero L0 adapter specific issues loader Loader related feature/bug native-cpu Native CPU adapter specific issues ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification v0.10.x Include in the v0.10.x release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants