[SYCL] Refactor memory objects to improve ABI stability (#1076)

This patch is a part of effort to decouple SYCL Runtime library
interface from its actual implementation. The goal is to improve
SYCL ABI/API compatibility between different versions of library.

The following changes were applied to SYCLMemObjT, as well as
buffer and image classes:

    Introduced a type-erased allocator. This allowed removal of
    templates for SYCLMemObjT, buffer_impl, and image_imp.
    sycl_mem_obj_t.hpp, image_impl.hpp, and buffer_impl.hpp
    were refactored to use public APIs only. All usages of private
    APIs were moved to cpp files.
    get_access member functions were moved from image_impl
    and buffer_impl to image and buffer accordingly, since they
    did not use any private APIs at all.


Signed-off-by: Alexander Batashev <[email protected]>

Author: Alexander Batashev

sycl/include/CL/sycl/buffer.hpp

@@ -50,71 +50,81 @@ class buffer {
   buffer(const range<dimensions> &bufferRange,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
-        get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList);
+    impl = std::make_shared<detail::buffer_impl>(
+        get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)), propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>());
   }
 
   buffer(const range<dimensions> &bufferRange, AllocatorT allocator,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)), propList,
-        allocator);
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   buffer(T *hostData, const range<dimensions> &bufferRange,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>());
   }
 
   buffer(T *hostData, const range<dimensions> &bufferRange,
          AllocatorT allocator, const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList, allocator);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   template <typename _T = T>
   buffer(EnableIfSameNonConstIterators<T, _T> const *hostData,
          const range<dimensions> &bufferRange,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>());
   }
 
   template <typename _T = T>
   buffer(EnableIfSameNonConstIterators<T, _T> const *hostData,
          const range<dimensions> &bufferRange, AllocatorT allocator,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList, allocator);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   buffer(const shared_ptr_class<T> &hostData,
          const range<dimensions> &bufferRange, AllocatorT allocator,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList, allocator);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   buffer(const shared_ptr_class<T> &hostData,
          const range<dimensions> &bufferRange,
          const property_list &propList = {})
       : Range(bufferRange) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         hostData, get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)),
-        propList);
+        propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>());
   }
 
   template <class InputIterator, int N = dimensions,
@@ -123,9 +133,11 @@ class buffer {
   buffer(InputIterator first, InputIterator last, AllocatorT allocator,
          const property_list &propList = {})
       : Range(range<1>(std::distance(first, last))) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         first, last, get_count() * sizeof(T),
-        detail::getNextPowerOfTwo(sizeof(T)), propList, allocator);
+        detail::getNextPowerOfTwo(sizeof(T)), propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   template <class InputIterator, int N = dimensions,
@@ -134,9 +146,10 @@ class buffer {
   buffer(InputIterator first, InputIterator last,
          const property_list &propList = {})
       : Range(range<1>(std::distance(first, last))) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         first, last, get_count() * sizeof(T),
-        detail::getNextPowerOfTwo(sizeof(T)), propList);
+        detail::getNextPowerOfTwo(sizeof(T)), propList,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>());
   }
 
   // This constructor is a prototype for a future SYCL specification
@@ -146,10 +159,11 @@ class buffer {
   buffer(Container &container, AllocatorT allocator,
          const property_list &propList = {})
       : Range(range<1>(container.size())) {
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
+    impl = std::make_shared<detail::buffer_impl>(
         container.data(), container.data() + container.size(),
         get_count() * sizeof(T), detail::getNextPowerOfTwo(sizeof(T)), propList,
-        allocator);
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(
+            allocator));
   }
 
   // This constructor is a prototype for a future SYCL specification
@@ -185,8 +199,10 @@ class buffer {
                           CL_MEM_SIZE, sizeof(size_t), &BufSize, nullptr);
 
     Range[0] = BufSize / sizeof(T);
-    impl = std::make_shared<detail::buffer_impl<AllocatorT>>(
-        MemObject, SyclContext, BufSize, AvailableEvent);
+    impl = std::make_shared<detail::buffer_impl>(
+        MemObject, SyclContext, BufSize,
+        make_unique_ptr<detail::SYCLMemObjAllocatorHolder<AllocatorT>>(),
+        AvailableEvent);
   }
 
   buffer(const buffer &rhs) = default;
@@ -213,38 +229,42 @@ class buffer {
 
   size_t get_size() const { return get_count() * sizeof(T); }
 
-  AllocatorT get_allocator() const { return impl->get_allocator(); }
+  AllocatorT get_allocator() const {
+    return impl->template get_allocator<AllocatorT>();
+  }
 
-  template <access::mode mode,
-            access::target target = access::target::global_buffer>
-  accessor<T, dimensions, mode, target, access::placeholder::false_t>
-  get_access(handler &commandGroupHandler) {
-    return impl->template get_access<T, dimensions, mode, target>(
-        *this, commandGroupHandler);
+  template <access::mode Mode,
+            access::target Target = access::target::global_buffer>
+  accessor<T, dimensions, Mode, Target, access::placeholder::false_t>
+  get_access(handler &CommandGroupHandler) {
+    return accessor<T, dimensions, Mode, Target, access::placeholder::false_t>(
+        *this, CommandGroupHandler);
   }
 
   template <access::mode mode>
   accessor<T, dimensions, mode, access::target::host_buffer,
            access::placeholder::false_t>
   get_access() {
-    return impl->template get_access<T, dimensions, mode>(*this);
+    return accessor<T, dimensions, mode, access::target::host_buffer,
+                    access::placeholder::false_t>(*this);
   }
 
   template <access::mode mode,
             access::target target = access::target::global_buffer>
   accessor<T, dimensions, mode, target, access::placeholder::false_t>
   get_access(handler &commandGroupHandler, range<dimensions> accessRange,
              id<dimensions> accessOffset = {}) {
-    return impl->template get_access<T, dimensions, mode, target>(
+    return accessor<T, dimensions, mode, target, access::placeholder::false_t>(
         *this, commandGroupHandler, accessRange, accessOffset);
   }
 
   template <access::mode mode>
   accessor<T, dimensions, mode, access::target::host_buffer,
            access::placeholder::false_t>
   get_access(range<dimensions> accessRange, id<dimensions> accessOffset = {}) {
-    return impl->template get_access<T, dimensions, mode>(*this, accessRange,
-                                                          accessOffset);
+    return accessor<T, dimensions, mode, access::target::host_buffer,
+                    access::placeholder::false_t>(*this, accessRange,
+                                                  accessOffset);
   }
 
   template <typename Destination = std::nullptr_t>
@@ -278,7 +298,7 @@ class buffer {
   }
 
 private:
-  shared_ptr_class<detail::buffer_impl<AllocatorT>> impl;
+  shared_ptr_class<detail::buffer_impl> impl;
   template <class Obj>
   friend decltype(Obj::impl) detail::getSyclObjImpl(const Obj &SyclObject);
   template <typename A, int dims, typename C> friend class buffer;
@@ -292,7 +312,7 @@ class buffer {
   bool IsSubBuffer = false;
 
   // Reinterpret contructor
-  buffer(shared_ptr_class<detail::buffer_impl<AllocatorT>> Impl,
+  buffer(shared_ptr_class<detail::buffer_impl> Impl,
          range<dimensions> reinterpretRange, size_t reinterpretOffset,
          bool isSubBuffer)
       : impl(Impl), Range(reinterpretRange), OffsetInBytes(reinterpretOffset),
@@ -381,7 +401,7 @@ template <typename T, int dimensions, typename AllocatorT>
 struct hash<cl::sycl::buffer<T, dimensions, AllocatorT>> {
   size_t
   operator()(const cl::sycl::buffer<T, dimensions, AllocatorT> &b) const {
-    return hash<std::shared_ptr<cl::sycl::detail::buffer_impl<AllocatorT>>>()(
+    return hash<std::shared_ptr<cl::sycl::detail::buffer_impl>>()(
         cl::sycl::detail::getSyclObjImpl(b));
   }
 };

sycl/include/CL/sycl/detail/buffer_impl.hpp

@@ -11,13 +11,8 @@
 #include <CL/cl.h>
 #include <CL/sycl/access/access.hpp>
 #include <CL/sycl/context.hpp>
-#include <CL/sycl/detail/context_impl.hpp>
-#include <CL/sycl/detail/aligned_allocator.hpp>
 #include <CL/sycl/detail/common.hpp>
 #include <CL/sycl/detail/helpers.hpp>
-#include <CL/sycl/detail/memory_manager.hpp>
-#include <CL/sycl/detail/pi.hpp>
-#include <CL/sycl/detail/scheduler/scheduler.hpp>
 #include <CL/sycl/detail/sycl_mem_obj_t.hpp>
 #include <CL/sycl/handler.hpp>
 #include <CL/sycl/property_list.hpp>
@@ -42,33 +37,35 @@ using buffer_allocator = detail::sycl_memory_object_allocator;
 
 namespace detail {
 
-template <typename AllocatorT>
-class buffer_impl final : public SYCLMemObjT<AllocatorT> {
-  using BaseT = SYCLMemObjT<AllocatorT>;
+class buffer_impl final : public SYCLMemObjT {
+  using BaseT = SYCLMemObjT;
   using typename BaseT::MemObjType;
 
 public:
   buffer_impl(size_t SizeInBytes, size_t RequiredAlign,
-              const property_list &Props, AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {}
+              const property_list &Props,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {}
 
   buffer_impl(void *HostData, size_t SizeInBytes, size_t RequiredAlign,
-              const property_list &Props, AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {
+              const property_list &Props,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {
     BaseT::handleHostData(HostData, RequiredAlign);
   }
 
   buffer_impl(const void *HostData, size_t SizeInBytes, size_t RequiredAlign,
-              const property_list &Props, AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {
+              const property_list &Props,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {
     BaseT::handleHostData(HostData, RequiredAlign);
   }
 
   template <typename T>
   buffer_impl(const shared_ptr_class<T> &HostData, const size_t SizeInBytes,
               size_t RequiredAlign, const property_list &Props,
-              AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {
     BaseT::handleHostData(HostData, RequiredAlign);
   }
 
@@ -79,8 +76,9 @@ class buffer_impl final : public SYCLMemObjT<AllocatorT> {
   template <class InputIterator>
   buffer_impl(EnableIfNotConstIterator<InputIterator> First, InputIterator Last,
               const size_t SizeInBytes, size_t RequiredAlign,
-              const property_list &Props, AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {
+              const property_list &Props,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {
     BaseT::handleHostData(First, Last, RequiredAlign);
     // TODO: There is contradiction in the spec, in one place it says
     // the data is not copied back at all if the buffer is construted
@@ -97,70 +95,21 @@ class buffer_impl final : public SYCLMemObjT<AllocatorT> {
   template <class InputIterator>
   buffer_impl(EnableIfConstIterator<InputIterator> First, InputIterator Last,
               const size_t SizeInBytes, size_t RequiredAlign,
-              const property_list &Props, AllocatorT Allocator = AllocatorT())
-      : BaseT(SizeInBytes, Props, Allocator) {
+              const property_list &Props,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator)
+      : BaseT(SizeInBytes, Props, std::move(Allocator)) {
     BaseT::handleHostData(First, Last, RequiredAlign);
   }
 
   buffer_impl(cl_mem MemObject, const context &SyclContext,
-              const size_t SizeInBytes, event AvailableEvent = {})
-      : BaseT(MemObject, SyclContext, SizeInBytes, std::move(AvailableEvent)) {}
-
-  template <typename T, int Dimensions, access::mode Mode,
-            access::target Target = access::target::global_buffer>
-  accessor<T, Dimensions, Mode, Target, access::placeholder::false_t>
-  get_access(buffer<T, Dimensions, AllocatorT> &Buffer,
-             handler &CommandGroupHandler) {
-    return accessor<T, Dimensions, Mode, Target, access::placeholder::false_t>(
-        Buffer, CommandGroupHandler);
-  }
-
-  template <typename T, int Dimensions, access::mode Mode>
-  accessor<T, Dimensions, Mode, access::target::host_buffer,
-           access::placeholder::false_t>
-  get_access(buffer<T, Dimensions, AllocatorT> &Buffer) {
-    return accessor<T, Dimensions, Mode, access::target::host_buffer,
-                    access::placeholder::false_t>(Buffer);
-  }
-
-  template <typename T, int dimensions, access::mode mode,
-            access::target target = access::target::global_buffer>
-  accessor<T, dimensions, mode, target, access::placeholder::false_t>
-  get_access(buffer<T, dimensions, AllocatorT> &Buffer,
-             handler &commandGroupHandler, range<dimensions> accessRange,
-             id<dimensions> accessOffset) {
-    return accessor<T, dimensions, mode, target, access::placeholder::false_t>(
-        Buffer, commandGroupHandler, accessRange, accessOffset);
-  }
-
-  template <typename T, int dimensions, access::mode mode>
-  accessor<T, dimensions, mode, access::target::host_buffer,
-           access::placeholder::false_t>
-  get_access(buffer<T, dimensions, AllocatorT> &Buffer,
-             range<dimensions> accessRange, id<dimensions> accessOffset) {
-    return accessor<T, dimensions, mode, access::target::host_buffer,
-                    access::placeholder::false_t>(Buffer, accessRange,
-                                                  accessOffset);
-  }
+              const size_t SizeInBytes,
+              unique_ptr_class<SYCLMemObjAllocator> Allocator,
+              event AvailableEvent)
+      : BaseT(MemObject, SyclContext, SizeInBytes, std::move(AvailableEvent),
+              std::move(Allocator)) {}
 
   void *allocateMem(ContextImplPtr Context, bool InitFromUserData,
-                    void *HostPtr, RT::PiEvent &OutEventToWait) override {
-
-    assert(!(InitFromUserData && HostPtr) &&
-           "Cannot init from user data and reuse host ptr provided "
-           "simultaneously");
-
-    void *UserPtr = InitFromUserData ? BaseT::getUserPtr() : HostPtr;
-
-    assert(!(nullptr == UserPtr && BaseT::useHostPtr() && Context->is_host()) &&
-           "Internal error. Allocating memory on the host "
-           "while having use_host_ptr property");
-
-    return MemoryManager::allocateMemBuffer(
-        std::move(Context), this, UserPtr, BaseT::MHostPtrReadOnly,
-        BaseT::getSize(), BaseT::MInteropEvent, BaseT::MInteropContext,
-        OutEventToWait);
-  }
+                   void *HostPtr, RT::PiEvent &OutEventToWait) override;
 
   MemObjType getType() const override { return MemObjType::BUFFER; }