Add test for usm buffer location properties #955
Conversation
sherry-yuan
force-pushed
the
usm_buffer_location
branch
2 times, most recently
from
877d182 to
7efc703
Compare
|
Test failures are in |
SYCL/USM/buffer_location.cpp
Outdated
| int *test_src_ptr = malloc_device<int>( | ||
| 1, dev, ctxt, | ||
| property_list{ext::intel::experimental::property::usm::buffer_location( | ||
| true_buf_loc)}); | ||
| int *test_dst_ptr = malloc_device<int>( | ||
| 1, dev, ctxt, | ||
| property_list{ext::intel::experimental::property::usm::buffer_location( | ||
| true_buf_loc)}); |
There was a problem hiding this comment.
These pointers leak. Either of SYCL API calls can throw an exception. Please, wrap them into std::unique_ptr.
There was a problem hiding this comment.
Thanks for reviewing:) I tried this suggestion but it seems like currently both unique_ptr or shareed_ptr are not device copyable (I think its because this poitner is used in the kernel). When I compiled, the following error were thrown:
error: static_assert failed due to requirement 'is_device_copyable<std::unique_ptr<int, std::default_delete<int>>, void>::value || detail::IsDeprecatedDeviceCopyable<std::unique_ptr<int, std::default_delete<int>>, void>::value' "The specified type is not device copyable"
static_assert(is_device_copyable<FieldT>::value ||
I thin it s fine to leave it at this point, because if there is an error, it is from the lower level runtime, which means the pointer is not successfully allocated. In that case, there wouldn't be any memory leaked.
There was a problem hiding this comment.
I improve the check a bit so that if it is only the second one failed, the first allocated memroy will not leak.
There was a problem hiding this comment.
@sherry-yuan you can use std::unique_ptr::get() to copy pointer on device and custom deleters to clean up the memory, see https://en.cppreference.com/w/cpp/memory/unique_ptr
There was a problem hiding this comment.
Thanks! the latest commit wraps the device pointer in a new struct with custom deleters.
Resolves intel/llvm#5750 The test intended to test changes in PR: intel/llvm#5634
sherry-yuan
force-pushed
the
usm_buffer_location
branch
from
bbf6e07 to
f0549c0
Compare
|
I have applied the suggestion and squashed the commits for a cleaner merge. |
Resolves intel/llvm#5750 The test intended to test changes in PR: intel/llvm#5634
…uite#955) Resolves intel#5750 The test intended to test changes in PR: intel#5634
Resolves intel/llvm#5750
The test is for PR: intel/llvm#5634