[SYCL] Move an accessor pointer to global_device space (#2044)

With this patch an accessor pointer to global buffer is moved from global space to global_device space. That is done to distinguish this pointer from those USM pointers, that are allocated global space or global_host space, in compile time. In addition to this change there are added explicit conversion operator from global_device to global space for multi_ptr class and implicit convertion for atomic class from global_device for global space. The last change isn't covered by specification published here: #1840 , but is required to pass atomic_api CTS. Signed-off-by: Dmitry Sidorov <[email protected]>
intel · Jul 10, 2020 · bc42582 · bc42582
1 parent efac3c2
commit bc42582
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 5 deletions.
diff --git a/sycl/include/CL/sycl/access/access.hpp b/sycl/include/CL/sycl/access/access.hpp
@@ -124,6 +124,11 @@ template <access::target accessTarget> struct TargetToAS {
       access::address_space::global_space;
 };
 
+template <> struct TargetToAS<access::target::global_buffer> {
+  constexpr static access::address_space AS =
+      access::address_space::global_device_space;
+};
+
 template <> struct TargetToAS<access::target::local> {
   constexpr static access::address_space AS =
       access::address_space::local_space;

diff --git a/sycl/include/CL/sycl/atomic.hpp b/sycl/include/CL/sycl/atomic.hpp
@@ -173,6 +173,7 @@ namespace sycl {
 template <typename T, access::address_space addressSpace =
                           access::address_space::global_space>
 class atomic {
+  friend class atomic<T, access::address_space::global_space>;
   static_assert(detail::IsValidAtomicType<T>::value,
                 "Invalid SYCL atomic type. Valid types are: int, "
                 "unsigned int, long, unsigned long, long long, unsigned "
@@ -197,6 +198,23 @@ class atomic {
                   "T and pointerT must be same size");
   }
 
+  // Create atomic in global_space with one from global_device_space
+  template <access::address_space _Space = addressSpace,
+            typename = typename std::enable_if<
+                _Space == addressSpace &&
+                addressSpace == access::address_space::global_space>::type>
+  atomic(const atomic<T, access::address_space::global_device_space> &RHS) {
+    Ptr = RHS.Ptr;
+  }
+
+  template <access::address_space _Space = addressSpace,
+            typename = typename std::enable_if<
+                _Space == addressSpace &&
+                addressSpace == access::address_space::global_space>::type>
+  atomic(atomic<T, access::address_space::global_device_space> &&RHS) {
+    Ptr = RHS.Ptr;
+  }
+
   void store(T Operand, memory_order Order = memory_order::relaxed) {
     __spirv_AtomicStore(
         Ptr, SpirvScope, detail::getSPIRVMemorySemanticsMask(Order), Operand);

diff --git a/sycl/include/CL/sycl/handler.hpp b/sycl/include/CL/sycl/handler.hpp
@@ -506,7 +506,7 @@ class __SYCL_EXPORT handler {
             access::placeholder IsPH>
   detail::enable_if_t<Dim == 0 && Mode == access::mode::atomic, T>
   readFromFirstAccElement(accessor<T, Dim, Mode, Target, IsPH> Src) const {
-    atomic<T, access::address_space::global_space> AtomicSrc = Src;
+    atomic<T, access::address_space::global_device_space> AtomicSrc = Src;
     return AtomicSrc.load();
   }
 
@@ -529,7 +529,7 @@ class __SYCL_EXPORT handler {
             access::placeholder IsPH>
   detail::enable_if_t<Dim == 0 && Mode == access::mode::atomic, void>
   writeToFirstAccElement(accessor<T, Dim, Mode, Target, IsPH> Dst, T V) const {
-    atomic<T, access::address_space::global_space> AtomicDst = Dst;
+    atomic<T, access::address_space::global_device_space> AtomicDst = Dst;
     AtomicDst.store(V);
   }
 

diff --git a/sycl/include/CL/sycl/intel/atomic_ref.hpp b/sycl/include/CL/sycl/intel/atomic_ref.hpp
@@ -43,7 +43,8 @@ using IsValidAtomicType =
 template <cl::sycl::access::address_space AS>
 using IsValidAtomicAddressSpace =
     bool_constant<AS == access::address_space::global_space ||
-                  AS == access::address_space::local_space>;
+                  AS == access::address_space::local_space ||
+                  AS == access::address_space::global_device_space>;
 
 // DefaultOrder parameter is limited to read-modify-write orders
 template <memory_order Order>
@@ -138,7 +139,7 @@ class atomic_ref_base {
                 "intel::atomic_ref does not yet support pointer types");
   static_assert(detail::IsValidAtomicAddressSpace<AddressSpace>::value,
                 "Invalid atomic address_space.  Valid address spaces are: "
-                "global_space, local_space");
+                "global_space, local_space, global_device_space");
   static_assert(
       detail::IsValidDefaultOrder<DefaultOrder>::value,
       "Invalid default memory_order for atomics.  Valid defaults are: "

diff --git a/sycl/include/CL/sycl/multi_ptr.hpp b/sycl/include/CL/sycl/multi_ptr.hpp
@@ -275,6 +275,22 @@ template <typename ElementType, access::address_space Space> class multi_ptr {
     return multi_ptr(m_Pointer - r);
   }
 
+  // Explicit conversion to global_space
+  // Only available if Space == address_space::global_device_space ||
+  // Space == address_space::global_host_space
+  template <access::address_space _Space = Space,
+            typename = typename std::enable_if<
+                _Space == Space &&
+                (Space == access::address_space::global_device_space ||
+                 Space == access::address_space::global_host_space)>::type>
+  explicit
+  operator multi_ptr<ElementType, access::address_space::global_space>() const {
+    using global_pointer_t = typename detail::PtrValueType<
+        ElementType, access::address_space::global_space>::type *;
+    return multi_ptr<ElementType, access::address_space::global_space>(
+        reinterpret_cast<global_pointer_t>(m_Pointer));
+  }
+
   // Only if Space == global_space
   template <access::address_space _Space = Space,
             typename = typename std::enable_if<

diff --git a/sycl/test/check_device_code/kernel_arguments_as.cpp b/sycl/test/check_device_code/kernel_arguments_as.cpp
@@ -5,7 +5,7 @@
 //
 // CHECK: %struct{{.*}}AccWrapper = type { %"class{{.*}}cl::sycl::accessor" }
 // CHECK: %"class{{.*}}cl::sycl::accessor" = type { %"class{{.*}}AccessorImplDevice", %[[UNION:.*]] }
-// CHECK: %[[UNION]] = type { i32 addrspace(1)* }
+// CHECK: %[[UNION]] = type { i32 addrspace(5)* }
 // CHECK: %struct{{.*}}AccWrapper = type { %"class{{.*}}cl::sycl::accessor" }
 // CHECK-NEXT: %"class{{.*}}cl::sycl::accessor" = type { %"class{{.*}}LocalAccessorBaseDevice", i32 addrspace(3)* }
 //

diff --git a/sycl/test/multi_ptr/multi_ptr.cpp b/sycl/test/multi_ptr/multi_ptr.cpp
@@ -98,6 +98,11 @@ template <typename T> void testMultPtr() {
         global_ptr<void> ptr_8 = global_ptr<void>(ptr_7);
         host_ptr<void> ptr_9((void *)RawPtr);
         global_ptr<void> ptr_10 = global_ptr<void>(ptr_9);
+        // TODO: need propagation of a7b763b26 patch to acl tool before testing
+        // these conversions - otherwise the test would fail on accelerator
+        // device during reversed translation from SPIR-V to LLVM IR
+        // device_ptr<T> ptr_11(accessorData_1);
+        // global_ptr<T> ptr_12 = global_ptr<T>(ptr_11);
 
         innerFunc<T>(wiID.get(0), ptr_1, ptr_2, local_ptr);
       });

diff --git a/sycl/test/regression/implicit_atomic_conversion.cpp b/sycl/test/regression/implicit_atomic_conversion.cpp
@@ -0,0 +1,29 @@
+// RUN: %clangxx -fsycl -fsycl-targets=%sycl_triple %s -o %t.out
+// RUN: env SYCL_DEVICE_TYPE=HOST %t.out
+// RUN: %CPU_RUN_PLACEHOLDER %t.out
+// RUN: %GPU_RUN_PLACEHOLDER %t.out
+
+#include <CL/sycl.hpp>
+
+using namespace cl::sycl;
+
+void test_conversion(queue q) {
+  int init = 0;
+  {
+    buffer<int> in_buf(&init, 1);
+
+    q.submit([&](handler &cgh) {
+      auto in = in_buf.template get_access<access::mode::atomic>(cgh);
+      cgh.single_task<class conversion>([=]() {
+        cl::sycl::atomic<int, access::address_space::global_space> atm = in[0];
+        atm.store(42);
+      });
+    });
+  }
+  assert(init == 42 && "verification failed");
+}
+
+int main() {
+  queue q;
+  test_conversion(q);
+}