From 6807c8cffaff3d76942ca12b2bf0530f4c43348d Mon Sep 17 00:00:00 2001
From: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
Date: Thu, 4 Apr 2019 14:58:06 +0300
Subject: [PATCH] [SYCL] Implement buffer constructor with iterators in
 accordance with spec

This change allows to use this constructor with constant iterators.
Data is written back to the same iterator set if the input iterator is not
a const iterator - in accordance with spec.

Signed-off-by: Mariya Podchishchaeva <mariya.podchishchaeva@intel.com>
---
 sycl/include/CL/sycl/detail/buffer_impl.hpp | 57 ++++++++++++++++-----
 sycl/test/basic_tests/buffer/buffer.cpp     | 36 ++++++++++---
 2 files changed, 71 insertions(+), 22 deletions(-)

diff --git a/sycl/include/CL/sycl/detail/buffer_impl.hpp b/sycl/include/CL/sycl/detail/buffer_impl.hpp
index 4da8134a9442..cc9d1ec302f1 100644
--- a/sycl/include/CL/sycl/detail/buffer_impl.hpp
+++ b/sycl/include/CL/sycl/detail/buffer_impl.hpp
@@ -99,23 +99,52 @@ template <typename AllocatorT> class buffer_impl {
     }
   }
 
+  template <typename Iterator> struct is_const_iterator {
+    using pointer = typename std::iterator_traits<Iterator>::pointer;
+    static constexpr bool value =
+        std::is_const<typename std::remove_pointer<pointer>::type>::value;
+  };
+
+  template <typename Iterator>
+  using EnableIfConstIterator =
+      typename std::enable_if<is_const_iterator<Iterator>::value,
+                              Iterator>::type;
+
+  template <typename Iterator>
+  using EnableIfNotConstIterator =
+      typename std::enable_if<!is_const_iterator<Iterator>::value,
+                              Iterator>::type;
+
   template <class InputIterator>
-  buffer_impl(InputIterator first, InputIterator last, const size_t sizeInBytes,
-              const property_list &propList,
+  buffer_impl(EnableIfNotConstIterator<InputIterator> first, InputIterator last,
+              const size_t sizeInBytes, const property_list &propList,
               AllocatorT allocator = AllocatorT())
       : SizeInBytes(sizeInBytes), Props(propList), MAllocator(allocator) {
-    if (Props.has_property<property::buffer::use_host_ptr>()) {
-      // TODO next line looks unsafe
-      BufPtr = &*first;
-    } else {
-      BufData.resize(get_size());
-      BufPtr = reinterpret_cast<void *>(BufData.data());
-      // We need cast BufPtr to pointer to the iteration type to get correct
-      // offset in std::copy when it will increment destination pointer.
-      auto *Ptr = reinterpret_cast<
-          typename std::iterator_traits<InputIterator>::pointer>(BufPtr);
-      std::copy(first, last, Ptr);
-    }
+    BufData.resize(get_size());
+    BufPtr = reinterpret_cast<void *>(BufData.data());
+    // We need cast BufPtr to pointer to the iteration type to get correct
+    // offset in std::copy when it will increment destination pointer.
+    auto *Ptr =
+        reinterpret_cast<typename std::iterator_traits<InputIterator>::pointer>(
+            BufPtr);
+    std::copy(first, last, Ptr);
+    // Data is written back if InputIterator is not a const iterator.
+    set_final_data(first);
+  }
+
+  template <class InputIterator>
+  buffer_impl(EnableIfConstIterator<InputIterator> first, InputIterator last,
+              const size_t sizeInBytes, const property_list &propList,
+              AllocatorT allocator = AllocatorT())
+      : SizeInBytes(sizeInBytes), Props(propList), MAllocator(allocator) {
+    BufData.resize(get_size());
+    BufPtr = reinterpret_cast<void *>(BufData.data());
+    // We need cast BufPtr to pointer to the iteration type to get correct
+    // offset in std::copy when it will increment destination pointer.
+    typedef typename std::iterator_traits<InputIterator>::value_type value;
+    auto *Ptr = reinterpret_cast<typename std::add_pointer<
+        typename std::remove_const<value>::type>::type>(BufPtr);
+    std::copy(first, last, Ptr);
   }
 
   buffer_impl(cl_mem MemObject, const context &SyclContext,
diff --git a/sycl/test/basic_tests/buffer/buffer.cpp b/sycl/test/basic_tests/buffer/buffer.cpp
index 574d938fee1d..9d8d6be64f7a 100644
--- a/sycl/test/basic_tests/buffer/buffer.cpp
+++ b/sycl/test/basic_tests/buffer/buffer.cpp
@@ -403,8 +403,29 @@ int main() {
             range<1>{3}, [=](id<1> index) { B[index] = 20; });
       });
     }
-    // Data is not copied back in the desctruction of the buffer created from
-    // pair of iterators
+    // Data is copied back in the desctruction of the buffer created from
+    // pair of non-const iterators
+    for (int i = 0; i < 2; i++)
+      assert(data1[i] == -1);
+    for (int i = 2; i < 5; i++)
+      assert(data1[i] == 20);
+    for (int i = 5; i < 10; i++)
+      assert(data1[i] == -1);
+  }
+
+  // Check that data is not copied back in the desctruction of the buffer
+  // created from pair of const iterators
+  {
+    std::vector<int> data1(10, -1);
+    {
+      buffer<int, 1> b(data1.cbegin() + 2, data1.cbegin() + 5);
+      queue myQueue;
+      myQueue.submit([&](handler &cgh) {
+        auto B = b.get_access<access::mode::read_write>(cgh);
+        cgh.parallel_for<class const_iter_constuctor>(
+            range<1>{3}, [=](id<1> index) { B[index] = 20; });
+      });
+    }
     for (int i = 0; i < 10; i++)
       assert(data1[i] == -1);
   }
@@ -435,9 +456,10 @@ int main() {
   // created from pair of iterators
   {
     std::vector<int> data1(10, -1);
+    std::vector<int> data2(10, -1);
     {
       buffer<int, 1> b(data1.begin() + 2, data1.begin() + 5);
-      b.set_final_data(data1.begin() + 2);
+      b.set_final_data(data2.begin() + 2);
       queue myQueue;
       myQueue.submit([&](handler &cgh) {
         auto B = b.get_access<access::mode::read_write>(cgh);
@@ -445,14 +467,12 @@ int main() {
             range<1>{3}, [=](id<1> index) { B[index] = 20; });
       });
     }
-    // Data is not copied back in the desctruction of the buffer created from
-    // pair of iterators
     for (int i = 0; i < 2; i++)
-      assert(data1[i] == -1);
+      assert(data2[i] == -1);
     for (int i = 2; i < 5; i++)
-      assert(data1[i] == 20);
+      assert(data2[i] == 20);
     for (int i = 5; i < 10; i++)
-      assert(data1[i] == -1);
+      assert(data2[i] == -1);
   }
 
   // Check that data is copied back after forcing write-back using