[SYCL][ABI-break] Promote guarded SYCL 2020 features and fix buffer reinterpret #6541
Conversation
…einterpret Promotes all SYCL 2020 features currently guarded by the SYCL2020_CONFORMANT_APIS macro. Additionally, `buffer::reinterpret` is changed to correctly rebind the allocator. To accomplish this, `std::allocator_traits` is specialized for `aligned_allocator`. Signed-off-by: Larsen, Steffen <[email protected]>
Signed-off-by: Larsen, Steffen <[email protected]>
|
/verify with intel/llvm-test-suite#1137 |
| be templated and `sycl::buffer` will be using `sycl::buffer_allocator<std::remove_const_t<T>>` | ||
| by default, where `T` is the data type of that buffer, if it is not explicitly given an | ||
| allocator. | ||
| Defining this macro currently has no effect on the API. |
There was a problem hiding this comment.
Should we remove it completely then? Maybe (hopefully) next use will be SYCL202Y and not 2020...
There was a problem hiding this comment.
The motivation for keeping it is so we can reuse it for any SYCL 2020 changes we may have missed that break API/ABI after we once again close the breakage window. We could always reintroduce it of course, but this way we should hopefully keep the same name for the macro as before if we run into a case like that.
The primary benefit is that users that do not worry about ABI and just want SYCL 2020 features as soon as they are ready can just keep defining this in their project and they will get it automatically as we add such missing features.
@pvchupin - What are your thoughts on this?
There was a problem hiding this comment.
I think the change proposed is the best at this point.
It would be great to track carefully documentation and implementation. It would be great to do everything in this window. But in reality I guess we will miss something or find problem late. So keeping macro around should be fine, as a good predefined/agreed name.
Signed-off-by: Larsen, Steffen <[email protected]>
| be templated and `sycl::buffer` will be using `sycl::buffer_allocator<std::remove_const_t<T>>` | ||
| by default, where `T` is the data type of that buffer, if it is not explicitly given an | ||
| allocator. | ||
| Defining this macro currently has no effect on the API. |
There was a problem hiding this comment.
I think the change proposed is the best at this point.
It would be great to track carefully documentation and implementation. It would be great to do everything in this window. But in reality I guess we will miss something or find problem late. So keeping macro around should be fine, as a good predefined/agreed name.
|
/verify with intel/llvm-test-suite#1137 |
|
@steffenlarsen, there've been HIP-unique device_num fail, is it related? |
With SYCL 2020 features that were previously guarded by the SYCL2020_CONFORMANT_APIS, tests that distinguished between behaviours with and without the macro are changed to expect the previously guarded API in all cases. Related change: intel/llvm#6541
Test was temporarily disabled on HIP (intel/llvm-test-suite#1136) due to consistent test failures. |
…1137) With SYCL 2020 features that were previously guarded by the SYCL2020_CONFORMANT_APIS, tests that distinguished between behaviours with and without the macro are changed to expect the previously guarded API in all cases. Related change: intel#6541
Promotes all SYCL 2020 features currently guarded by the SYCL2020_CONFORMANT_APIS macro.
Additionally,
buffer::reinterpretis changed to correctly rebind the allocator. To accomplish this,std::allocator_traitsis specialized foraligned_allocator.Related change: intel/llvm-test-suite#1137