[SYCLomatic] Add malloc, free, and temporary allocation mappings #1165
Conversation
Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
…ect conversion for const tagged_pointer types Signed-off-by: Matthew Michel <[email protected]>
There was a problem hiding this comment.
Pls add test case in https://github.com/oneapi-src/SYCLomatic-test/tree/SYCLomatic/help_function/src for new added helper APIs in this PR
| class tagged_pointer_base { | ||
| public: | ||
| using pointer = T *; | ||
| using difference_type = ::std::make_signed_t<std::size_t>; |
There was a problem hiding this comment.
I have switched to using this.
| public: | ||
| using pointer = T *; | ||
| using difference_type = ::std::make_signed_t<std::size_t>; | ||
| tagged_pointer_base() = default; |
There was a problem hiding this comment.
m_ptr is not initialized.
| tagged_pointer() : super_t() {} | ||
| tagged_pointer(T *ptr) : super_t(ptr) {} | ||
| T &operator[](difference_type idx) { return this->m_ptr[idx]; } | ||
| T &operator[](difference_type idx) const { return this->m_ptr[idx]; } |
There was a problem hiding this comment.
Should return const T & here?
There was a problem hiding this comment.
Yes, I have made this change.
| tagged_pointer &operator-=(difference_type backward) { | ||
| this->m_ptr = this->m_ptr - backward; | ||
| return *this; | ||
| } |
There was a problem hiding this comment.
operator* and operator-> and pointer type conversion are also needed?
There was a problem hiding this comment.
I have added this, so we are not relying on conversion to pointer. I also updated the test to check operator* and operator->.
There was a problem hiding this comment.
Do we need some type conversion like:
template<class OtherT>
operator tagged_pointer<Tag, OtherT>();
There was a problem hiding this comment.
I don't think this operator is necessary since we already provide a raw pointer conversion to otherT which can then call the tagged_pointer<Tag, OtherT> constructor. This functionality is tested in onedpl_test_tagged_pointer in the test PR.
We also don't provide this operator in similar classes (ex: device_pointer).
| oneapi::dpl::execution::is_execution_policy_v<decayed_policy_or_tag_t>; | ||
| static constexpr bool is_sys_tag_v = | ||
| ::std::is_base_of_v<sys_tag, std::decay_t<decayed_policy_or_tag_t>>; | ||
| static_assert(is_policy_v || is_sys_tag_v, |
There was a problem hiding this comment.
Is user-defined tag that deriving sys_tag also permitted?
There was a problem hiding this comment.
Since we do not currently support any custom execution policies in oneDPL, I think we should not currently allow user-defined system tags. I have put a stricter check in here to ensure this.
|
|
||
| // TODO: Improve this pointer wrapper and make an iterator adaptor | ||
| // once we have the Boost library dependency | ||
| template <typename Tag, typename T, typename Derived> |
There was a problem hiding this comment.
Why we need this base class?
There was a problem hiding this comment.
This base class just contains the shared functions / members between the standard tagged_pointer case and the specialization for void pointers. The CRTP template parameter is currently unused. I have removed it.
There was a problem hiding this comment.
The base class just provide two simple methods:
operator const T *() const { return m_ptr; }
operator T *() { return m_ptr; }
I think we can remove it.
There was a problem hiding this comment.
I have made this change.
Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
…s_tag types Signed-off-by: Matthew Michel <[email protected]>
Signed-off-by: Matthew Michel <[email protected]>
|
@tomflinda I have added tests for the system tag internal helper APIs in: https://github.com/oneapi-src/SYCLomatic-test/pull/427/files and a test for the |
There was a problem hiding this comment.
LGTM, it would be good to get and approval from @ziranzha before merging.
Signed-off-by: Matthew Michel <[email protected]>
…on tests (oneapi-src#426) Signed-off-by: Matthew Michel <[email protected]>
) Signed-off-by: Matthew Michel <[email protected]>
In this PR, we add the APIs for
malloc,free, and temporary buffers (get_temporary_allocationandrelease_temporary_allocation). Atagged_pointertype is added that contains a template parameter of typehost_sys_tagordevice_sys_tagspecifying the location of the allocation. This type is returned bymallocandget_temporary_allocation.tagged_pointercurrently provides basic pointer functionality and will be improved in the future once we get access to Boost iterator adaptors.In addition to passing execution policies to the memory allocation APIs, passing a system tag type is also supported in this PR. The implemented functionality is only supported for USM mode. Static asserts are hit if
DPCT_USM_LEVEL_NONEis set.There are two PRs that test this functionality: