Merge pull request #1061 from IntelPython/fix/DPCTLUSM_GetPointerType

Changes the API for DPCTLUSM_GetPointerType to return an enum.

Author: Diptorup Deb

dpctl/_backend.pxd

@@ -34,6 +34,12 @@ cdef extern from "syclinterface/dpctl_utils.h":
 
 
 cdef extern from "syclinterface/dpctl_sycl_enum_types.h":
+    ctypedef enum _usm_type 'DPCTLSyclUSMType':
+        _USM_UNKNOWN     'DPCTL_USM_UNKNOWN'
+        _USM_DEVICE      'DPCTL_USM_DEVICE'
+        _USM_HOST        'DPCTL_USM_HOST'
+        _USM_SHARED      'DPCTL_USM_SHARED'
+
     ctypedef enum _backend_type 'DPCTLSyclBackendType':
         _ALL_BACKENDS    'DPCTL_ALL_BACKENDS'
         _CUDA            'DPCTL_CUDA'
@@ -340,7 +346,8 @@ cdef extern from "syclinterface/dpctl_sycl_kernel_bundle_interface.h":
     cdef bool DPCTLKernelBundle_HasKernel(DPCTLSyclKernelBundleRef KBRef,
                                      const char *KernelName)
     cdef void DPCTLKernelBundle_Delete(DPCTLSyclKernelBundleRef KBRef)
-    cdef DPCTLSyclKernelBundleRef DPCTLKernelBundle_Copy(const DPCTLSyclKernelBundleRef KBRef)
+    cdef DPCTLSyclKernelBundleRef DPCTLKernelBundle_Copy(
+        const DPCTLSyclKernelBundleRef KBRef)
 
 
 cdef extern from "syclinterface/dpctl_sycl_queue_interface.h":
@@ -450,7 +457,7 @@ cdef extern from "syclinterface/dpctl_sycl_usm_interface.h":
     cdef void DPCTLfree_with_context(
         DPCTLSyclUSMRef MRef,
         DPCTLSyclContextRef CRef)
-    cdef const char* DPCTLUSM_GetPointerType(
+    cdef _usm_type DPCTLUSM_GetPointerType(
         DPCTLSyclUSMRef MRef,
         DPCTLSyclContextRef CRef)
     cdef DPCTLSyclDeviceRef DPCTLUSM_GetPointerDevice(

dpctl/memory/_memory.pyx

@@ -55,6 +55,7 @@ from dpctl._backend cimport (  # noqa: E211
     DPCTLSyclUSMRef,
     DPCTLUSM_GetPointerDevice,
     DPCTLUSM_GetPointerType,
+    _usm_type,
 )
 
 from .._sycl_context cimport SyclContext
@@ -232,10 +233,10 @@ cdef class _Memory:
     cdef _getbuffer(self, Py_buffer *buffer, int flags):
         # memory_ptr is Ref which is pointer to SYCL type. For USM it is void*.
         cdef SyclContext ctx = self._context
-        cdef const char *kind = DPCTLUSM_GetPointerType(
+        cdef _usm_type UsmTy = DPCTLUSM_GetPointerType(
             self.memory_ptr, ctx.get_context_ref()
         )
-        if kind == b'device':
+        if UsmTy == _usm_type._USM_DEVICE:
             raise ValueError("USM Device memory is not host accessible")
         buffer.buf = <char*>self.memory_ptr
         buffer.format = 'B'                     # byte
@@ -547,11 +548,17 @@ cdef class _Memory:
             using the given context. Otherwise, returns b'shared', b'device',
             or b'host' type of the allocation.
         """
-        cdef const char * usm_type = DPCTLUSM_GetPointerType(
+        cdef _usm_type usm_ty = DPCTLUSM_GetPointerType(
             p, ctx.get_context_ref()
         )
-
-        return <bytes>usm_type
+        if usm_ty == _usm_type._USM_DEVICE:
+            return b'device'
+        elif usm_ty == _usm_type._USM_HOST:
+            return b'host'
+        elif usm_ty == _usm_type._USM_SHARED:
+            return b'shared'
+        else:
+            return b'unknown'
 
     @staticmethod
     cdef object create_from_usm_pointer_size_qref(
@@ -569,7 +576,7 @@ cdef class _Memory:
         The object may not be a no-op dummy Python object to
         delay freeing the memory until later times.
         """
-        cdef const char *usm_type
+        cdef _usm_type usm_ty
         cdef DPCTLSyclContextRef CRef = NULL
         cdef DPCTLSyclQueueRef QRef_copy = NULL
         cdef _Memory _mem
@@ -581,13 +588,13 @@ cdef class _Memory:
         CRef = DPCTLQueue_GetContext(QRef)
         if (CRef is NULL):
             raise ValueError("Could not retrieve context from QRef")
-        usm_type = DPCTLUSM_GetPointerType(USMRef, CRef)
+        usm_ty = DPCTLUSM_GetPointerType(USMRef, CRef)
         DPCTLContext_Delete(CRef)
-        if usm_type == b"shared":
+        if usm_ty == _usm_type._USM_SHARED:
             mem_ty = MemoryUSMShared
-        elif usm_type == b"device":
+        elif usm_ty == _usm_type._USM_DEVICE:
             mem_ty = MemoryUSMDevice
-        elif usm_type == b"host":
+        elif usm_ty == _usm_type._USM_HOST:
             mem_ty = MemoryUSMHost
         else:
             raise ValueError(

libsyclinterface/include/dpctl_sycl_enum_types.h

@@ -31,6 +31,18 @@
 
 DPCTL_C_EXTERN_C_BEGIN
 
+/*!
+ * @brief Enum types for SYCL's USM allocator types.
+ *
+ */
+typedef enum
+{
+    DPCTL_USM_UNKNOWN = 0,
+    DPCTL_USM_DEVICE,
+    DPCTL_USM_HOST,
+    DPCTL_USM_SHARED
+} DPCTLSyclUSMType;
+
 /*!
  * @brief Redefinition of DPC++-specific Sycl backend types.
  *

libsyclinterface/include/dpctl_sycl_usm_interface.h

@@ -29,6 +29,7 @@
 #include "Support/ExternC.h"
 #include "Support/MemOwnershipAttrs.h"
 #include "dpctl_data_types.h"
+#include "dpctl_sycl_enum_types.h"
 #include "dpctl_sycl_types.h"
 
 DPCTL_C_EXTERN_C_BEGIN
@@ -152,16 +153,17 @@ void DPCTLfree_with_context(__dpctl_take DPCTLSyclUSMRef MRef,
                             __dpctl_keep const DPCTLSyclContextRef CRef);
 
 /*!
- * @brief Get pointer type.
+ * @brief Returns the USM allocator type for a pointer.
  *
- * @param    MRef      USM Memory
+ * @param    MRef      USM allocated pointer
  * @param    CRef      Sycl context reference associated with the pointer
  *
- * @return "host", "device", "shared" or "unknown"
+ * @return DPCTLSyclUSMType enum value indicating if the pointer is of USM type
+ *         "shared", "host", or "device".
  * @ingroup USMInterface
  */
 DPCTL_API
-const char *
+DPCTLSyclUSMType
 DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
                         __dpctl_keep const DPCTLSyclContextRef CRef);
 

libsyclinterface/source/dpctl_sycl_usm_interface.cpp

@@ -177,30 +177,31 @@ void DPCTLfree_with_context(__dpctl_take DPCTLSyclUSMRef MRef,
     free(Ptr, *C);
 }
 
-const char *DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
-                                    __dpctl_keep const DPCTLSyclContextRef CRef)
+DPCTLSyclUSMType
+DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
+                        __dpctl_keep const DPCTLSyclContextRef CRef)
 {
     if (!CRef) {
         error_handler("Input CRef is nullptr.", __FILE__, __func__, __LINE__);
-        return "unknown";
+        return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
     }
     if (!MRef) {
         error_handler("Input MRef is nullptr.", __FILE__, __func__, __LINE__);
-        return "unknown";
+        return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
     }
     auto Ptr = unwrap<void>(MRef);
     auto C = unwrap<context>(CRef);
 
     auto kind = get_pointer_type(Ptr, *C);
     switch (kind) {
     case usm::alloc::host:
-        return "host";
+        return DPCTLSyclUSMType::DPCTL_USM_HOST;
     case usm::alloc::device:
-        return "device";
+        return DPCTLSyclUSMType::DPCTL_USM_DEVICE;
     case usm::alloc::shared:
-        return "shared";
+        return DPCTLSyclUSMType::DPCTL_USM_SHARED;
     default:
-        return "unknown";
+        return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
     }
 }
 

libsyclinterface/tests/test_sycl_usm_interface.cpp

@@ -45,12 +45,12 @@ constexpr size_t SIZE = 1024;
 void common_test_body(size_t nbytes,
                       const DPCTLSyclUSMRef Ptr,
                       const DPCTLSyclQueueRef Q,
-                      const char *expected)
+                      DPCTLSyclUSMType expected)
 {
     auto Ctx = DPCTLQueue_GetContext(Q);
 
     auto kind = DPCTLUSM_GetPointerType(Ptr, Ctx);
-    EXPECT_TRUE(0 == std::strncmp(kind, expected, 4));
+    EXPECT_TRUE(kind == expected);
 
     auto Dev = DPCTLUSM_GetPointerDevice(Ptr, Ctx);
     auto QueueDev = DPCTLQueue_GetDevice(Q);
@@ -100,7 +100,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocShared)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLmalloc_shared(nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "shared");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_SHARED);
     DPCTLfree_with_queue(Ptr, Q);
     DPCTLQueue_Delete(Q);
 }
@@ -112,7 +112,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocDevice)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLmalloc_device(nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "device");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_DEVICE);
     DPCTLfree_with_queue(Ptr, Q);
     DPCTLQueue_Delete(Q);
 }
@@ -124,7 +124,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocHost)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLmalloc_host(nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "host");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_HOST);
     DPCTLfree_with_queue(Ptr, Q);
     DPCTLQueue_Delete(Q);
 }
@@ -136,7 +136,7 @@ TEST_F(TestDPCTLSyclUSMInterface, AlignedAllocShared)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLaligned_alloc_shared(64, nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "shared");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_SHARED);
     DPCTLfree_with_queue(Ptr, Q);
     DPCTLQueue_Delete(Q);
 }
@@ -148,7 +148,7 @@ TEST_F(TestDPCTLSyclUSMInterface, AlignedAllocDevice)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLaligned_alloc_device(64, nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "device");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_DEVICE);
     DPCTLfree_with_queue(Ptr, Q);
     DPCTLQueue_Delete(Q);
 }
@@ -160,7 +160,7 @@ TEST_F(TestDPCTLSyclUSMInterface, AlignedAllocHost)
     const size_t nbytes = SIZE;
     auto Ptr = DPCTLaligned_alloc_host(64, nbytes, Q);
     EXPECT_TRUE(bool(Ptr));
-    common_test_body(nbytes, Ptr, Q, "host");
+    common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::DPCTL_USM_HOST);
     DPCTLfree_with_queue(Ptr, Q);
 }
 
@@ -234,13 +234,10 @@ TEST_F(TestDPCTLSyclUSMNullArgs, ChkPointerQueries)
 {
     DPCTLSyclContextRef Null_CRef = nullptr;
     DPCTLSyclUSMRef Null_MRef = nullptr;
-    const char *t = nullptr;
-    auto is_unknown = [=](const char *t) -> bool {
-        return strncmp(t, "unknown", 7) == 0;
-    };
+    DPCTLSyclUSMType t = DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
 
     EXPECT_NO_FATAL_FAILURE(t = DPCTLUSM_GetPointerType(Null_MRef, Null_CRef));
-    ASSERT_TRUE(is_unknown(t));
+    ASSERT_TRUE(t == DPCTLSyclUSMType::DPCTL_USM_UNKNOWN);
 
     DPCTLSyclDeviceSelectorRef DSRef = nullptr;
     DPCTLSyclDeviceRef DRef = nullptr;
@@ -253,9 +250,8 @@ TEST_F(TestDPCTLSyclUSMNullArgs, ChkPointerQueries)
     EXPECT_NO_FATAL_FAILURE(CRef = DPCTLContext_Create(DRef, nullptr, 0));
     EXPECT_NO_FATAL_FAILURE(DPCTLDevice_Delete(DRef));
 
-    t = nullptr;
     EXPECT_NO_FATAL_FAILURE(t = DPCTLUSM_GetPointerType(Null_MRef, CRef));
-    ASSERT_TRUE(is_unknown(t));
+    ASSERT_TRUE(t == DPCTLSyclUSMType::DPCTL_USM_UNKNOWN);
 
     DPCTLSyclDeviceRef D2Ref = nullptr;
     EXPECT_NO_FATAL_FAILURE(D2Ref = DPCTLUSM_GetPointerDevice(Null_MRef, CRef));