-
Notifications
You must be signed in to change notification settings - Fork 796
[SYCL] Fixes for usm_allocator properties
#19890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Fix `verifyUSMAllocatorProperties` to allow supported properties * No need to specialize `is_property<...>` for USM properties - `std::base_of_v<>`-based implementation covers that.
a5862bc to
8c1c2ac
Compare
| sycl::usm_allocator<int, sycl::usm::alloc::shared> allocator{ | ||
| q, | ||
| {sycl::ext::oneapi::property::usm::device_read_only{}, | ||
| sycl::ext::intel::experimental::property::usm::buffer_location{1}}}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some E2E tests for these properties (see for example Adapters/level_zero/usm_device_read_only.cpp and USM/buffer_location.cpp). Could these get some test cases with the allocator as well? Primarily I want to make sure the properties are being passed correctly to the allocations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the device_read_only part, but the buffer_location test doesn't verify anything (and is poorly written) and that is an FPGA extension, and we've deprecated whole FPGA support anyway.
b17879a to
1ae0565
Compare
|
Ping @cperkinsintel . Note that @steffenlarsen is OOO till Monday so he won't be able to review this, I think. |
* Fix `verifyUSMAllocatorProperties` to allow supported properties * No need to specialize `is_property<...>` for USM properties - `std::base_of_v<>`-based implementation covers that.
This is a cherry-pick of #19890 * Fix `verifyUSMAllocatorProperties` to allow supported properties * No need to specialize `is_property<...>` for USM properties - `std::base_of_v<>`-based implementation covers that. Patch-by: Andrei Elovikov <[email protected]>
verifyUSMAllocatorPropertiesto allow supported propertiesis_property<...>for USM properties -std::base_of_v<>-based implementation covers that.