[SYCL][USM] Add support for pointer query APIs #1016
Conversation
|
I'm thinking this is a driver version issue again. Can we update the drivers to the latest for GPU? |
| /// | ||
| /// @param ptr is the USM pointer to query | ||
| /// @param ctxt is the sycl context the ptr was allocated in | ||
| alloc get_pointer_type(const void *Ptr, const context &Ctxt) { |
There was a problem hiding this comment.
What if user mixes up context used for USM allocation and context passed to get_pointer_type function?
sycl::context Ctx1(sycl::cpu_selector), Ctx2(sycl::host_selector);
auto *Ptr = sycl::alloc_shared(Ctx1, ...);
auto Type = sycl::get_pointer_type(Ptr, Ctx2); // <- This returns alloc::host.
Can we add some diagnostic to catch such issues?
There was a problem hiding this comment.
The PI_CALL should fail if something like that goes wrong.
There was a problem hiding this comment.
In the example above get_pointer_type doesn't call PI.
There was a problem hiding this comment.
I'm not sure I follow.
There was a problem hiding this comment.
I mean, the code snippet doesn't return any error, while being illegal because Ptr is allocated in the Ctx1 but get_pointer_type is called with Ctx2.
There was a problem hiding this comment.
I'm not sure this can really be handled. In this case, I would expect it to just work as shared allocations work on the host and the host doesn't really care what context anything is in. CPU memory is CPU memory.
The trickier case is:
sycl::context Ctx1(sycl::gpu_selector), Ctx2(sycl::host_selector);
auto *Ptr = sycl::alloc_device(Ctx1, ...);
auto Type = sycl::get_pointer_type(Ptr, Ctx2); // <- This returns alloc::host.
Trying to use Ptr on the host ought to crash the program or give garbage values. We would have to keep track of insane amounts of state to try to get a host device context to bail on this.
There was a problem hiding this comment.
I think we should invent some diagnostic, probably enabled if some option is set only.
People facing this issue in real code already.
There was a problem hiding this comment.
I'm OK if done as a separate patch.
jbrodman
force-pushed
the
getpointerinfo2
branch
from
e1387e2 to
2f5ea9f
Compare
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
Signed-off-by: James Brodman <[email protected]>
jbrodman
force-pushed
the
getpointerinfo2
branch
from
2f5ea9f to
a164b01
Compare
Signed-off-by: James Brodman <[email protected]>
|
The test failures remain an "old" driver problem. |
Probably we should disable them for some time as well. |
Signed-off-by: James Brodman <[email protected]>
|
+@tfzhu to fix "old driver" issue. |
Fails on 19.48.14977 Works on 20.01.15265+ Didn't have any in-between versions handy to try. Windows, of course, will probably lag behind by even more. |
Working on uplift the RT version. Blocked by Windows lit test failure |
This PR adds support for APIs that query properties of USM pointers.
In particular, this PR enables asking the allocation type of a USM pointer (host/device/shared) and the device against which it was allocated (if applicable).
Host allocations return host devices.
Host devices treat all pointers as host allocs.
This PR replaces #902