Skip to content

Commit 86be4cf

Browse files
author
Diptorup Deb
committed
Changes the API for DPCTLUSM_GetPointerType.
- DPCTLUSM_GetPointerType used to return a global string literal and had an API that is different from all other libsyclinterface functions that return a C string. Returning a pointer to a global literal as opposed to a heap allocated memory meant the ownership of the pointer was not clear when crossing language boundary. Calling DPCTLCString_Delete on the previously returned const char* will cause a segfault - The PR introduces an enum for usm types and returns an enum value from DPCTLUSM_GetPointerType. - All Python API is unaffected.
1 parent d77f570 commit 86be4cf

File tree

6 files changed

+57
-28
lines changed

6 files changed

+57
-28
lines changed

dpctl/_backend.pxd

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ cdef extern from "syclinterface/dpctl_utils.h":
3434

3535

3636
cdef extern from "syclinterface/dpctl_sycl_enum_types.h":
37+
ctypedef enum _usm_type 'DPCTLSyclUSMType':
38+
_USM_UNKNOWN 'DPCTL_USM_UNKNOWN'
39+
_USM_DEVICE 'DPCTL_USM_DEVICE'
40+
_USM_HOST 'DPCTL_USM_HOST'
41+
_USM_SHARED 'DPCTL_USM_SHARED'
42+
3743
ctypedef enum _backend_type 'DPCTLSyclBackendType':
3844
_ALL_BACKENDS 'DPCTL_ALL_BACKENDS'
3945
_CUDA 'DPCTL_CUDA'
@@ -340,7 +346,8 @@ cdef extern from "syclinterface/dpctl_sycl_kernel_bundle_interface.h":
340346
cdef bool DPCTLKernelBundle_HasKernel(DPCTLSyclKernelBundleRef KBRef,
341347
const char *KernelName)
342348
cdef void DPCTLKernelBundle_Delete(DPCTLSyclKernelBundleRef KBRef)
343-
cdef DPCTLSyclKernelBundleRef DPCTLKernelBundle_Copy(const DPCTLSyclKernelBundleRef KBRef)
349+
cdef DPCTLSyclKernelBundleRef DPCTLKernelBundle_Copy(
350+
const DPCTLSyclKernelBundleRef KBRef)
344351

345352

346353
cdef extern from "syclinterface/dpctl_sycl_queue_interface.h":
@@ -450,7 +457,7 @@ cdef extern from "syclinterface/dpctl_sycl_usm_interface.h":
450457
cdef void DPCTLfree_with_context(
451458
DPCTLSyclUSMRef MRef,
452459
DPCTLSyclContextRef CRef)
453-
cdef const char* DPCTLUSM_GetPointerType(
460+
cdef _usm_type DPCTLUSM_GetPointerType(
454461
DPCTLSyclUSMRef MRef,
455462
DPCTLSyclContextRef CRef)
456463
cdef DPCTLSyclDeviceRef DPCTLUSM_GetPointerDevice(

dpctl/memory/_memory.pyx

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ from dpctl._backend cimport ( # noqa: E211
5555
DPCTLSyclUSMRef,
5656
DPCTLUSM_GetPointerDevice,
5757
DPCTLUSM_GetPointerType,
58+
_usm_type,
5859
)
5960

6061
from .._sycl_context cimport SyclContext
@@ -232,10 +233,10 @@ cdef class _Memory:
232233
cdef _getbuffer(self, Py_buffer *buffer, int flags):
233234
# memory_ptr is Ref which is pointer to SYCL type. For USM it is void*.
234235
cdef SyclContext ctx = self._context
235-
cdef const char *kind = DPCTLUSM_GetPointerType(
236+
cdef _usm_type UsmTy = DPCTLUSM_GetPointerType(
236237
self.memory_ptr, ctx.get_context_ref()
237238
)
238-
if kind == b'device':
239+
if UsmTy == _usm_type._USM_DEVICE:
239240
raise ValueError("USM Device memory is not host accessible")
240241
buffer.buf = <char*>self.memory_ptr
241242
buffer.format = 'B' # byte
@@ -547,11 +548,17 @@ cdef class _Memory:
547548
using the given context. Otherwise, returns b'shared', b'device',
548549
or b'host' type of the allocation.
549550
"""
550-
cdef const char * usm_type = DPCTLUSM_GetPointerType(
551+
cdef _usm_type usm_ty = DPCTLUSM_GetPointerType(
551552
p, ctx.get_context_ref()
552553
)
553-
554-
return <bytes>usm_type
554+
if usm_ty == _usm_type._USM_DEVICE:
555+
return b'device'
556+
elif usm_ty == _usm_type._USM_HOST:
557+
return b'host'
558+
elif usm_ty == _usm_type._USM_SHARED:
559+
return b'shared'
560+
else:
561+
return b'unknown'
555562

556563
@staticmethod
557564
cdef object create_from_usm_pointer_size_qref(
@@ -569,7 +576,7 @@ cdef class _Memory:
569576
The object may not be a no-op dummy Python object to
570577
delay freeing the memory until later times.
571578
"""
572-
cdef const char *usm_type
579+
cdef _usm_type usm_ty
573580
cdef DPCTLSyclContextRef CRef = NULL
574581
cdef DPCTLSyclQueueRef QRef_copy = NULL
575582
cdef _Memory _mem
@@ -581,13 +588,13 @@ cdef class _Memory:
581588
CRef = DPCTLQueue_GetContext(QRef)
582589
if (CRef is NULL):
583590
raise ValueError("Could not retrieve context from QRef")
584-
usm_type = DPCTLUSM_GetPointerType(USMRef, CRef)
591+
usm_ty = DPCTLUSM_GetPointerType(USMRef, CRef)
585592
DPCTLContext_Delete(CRef)
586-
if usm_type == b"shared":
593+
if usm_ty == _usm_type._USM_SHARED:
587594
mem_ty = MemoryUSMShared
588-
elif usm_type == b"device":
595+
elif usm_ty == _usm_type._USM_DEVICE:
589596
mem_ty = MemoryUSMDevice
590-
elif usm_type == b"host":
597+
elif usm_ty == _usm_type._USM_HOST:
591598
mem_ty = MemoryUSMHost
592599
else:
593600
raise ValueError(

libsyclinterface/include/dpctl_sycl_enum_types.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,18 @@
3131

3232
DPCTL_C_EXTERN_C_BEGIN
3333

34+
/*!
35+
* @brief Enum types for SYCL's USM allocator types.
36+
*
37+
*/
38+
typedef enum
39+
{
40+
DPCTL_USM_UNKNOWN = 0,
41+
DPCTL_USM_DEVICE,
42+
DPCTL_USM_HOST,
43+
DPCTL_USM_SHARED
44+
} DPCTLSyclUSMType;
45+
3446
/*!
3547
* @brief Redefinition of DPC++-specific Sycl backend types.
3648
*

libsyclinterface/include/dpctl_sycl_usm_interface.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "Support/ExternC.h"
3030
#include "Support/MemOwnershipAttrs.h"
3131
#include "dpctl_data_types.h"
32+
#include "dpctl_sycl_enum_types.h"
3233
#include "dpctl_sycl_types.h"
3334

3435
DPCTL_C_EXTERN_C_BEGIN
@@ -152,16 +153,17 @@ void DPCTLfree_with_context(__dpctl_take DPCTLSyclUSMRef MRef,
152153
__dpctl_keep const DPCTLSyclContextRef CRef);
153154

154155
/*!
155-
* @brief Get pointer type.
156+
* @brief Returns the USM allocator type for a pointer.
156157
*
157-
* @param MRef USM Memory
158+
* @param MRef USM allocated pointer
158159
* @param CRef Sycl context reference associated with the pointer
159160
*
160-
* @return "host", "device", "shared" or "unknown"
161+
* @return DPCTLSyclUSMType enum value indicating if the pointer is of USM type
162+
* "shared", "host", or "device".
161163
* @ingroup USMInterface
162164
*/
163165
DPCTL_API
164-
const char *
166+
DPCTLSyclUSMType
165167
DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
166168
__dpctl_keep const DPCTLSyclContextRef CRef);
167169

libsyclinterface/source/dpctl_sycl_usm_interface.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,30 +177,31 @@ void DPCTLfree_with_context(__dpctl_take DPCTLSyclUSMRef MRef,
177177
free(Ptr, *C);
178178
}
179179

180-
const char *DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
181-
__dpctl_keep const DPCTLSyclContextRef CRef)
180+
DPCTLSyclUSMType
181+
DPCTLUSM_GetPointerType(__dpctl_keep const DPCTLSyclUSMRef MRef,
182+
__dpctl_keep const DPCTLSyclContextRef CRef)
182183
{
183184
if (!CRef) {
184185
error_handler("Input CRef is nullptr.", __FILE__, __func__, __LINE__);
185-
return "unknown";
186+
return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
186187
}
187188
if (!MRef) {
188189
error_handler("Input MRef is nullptr.", __FILE__, __func__, __LINE__);
189-
return "unknown";
190+
return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
190191
}
191192
auto Ptr = unwrap<void>(MRef);
192193
auto C = unwrap<context>(CRef);
193194

194195
auto kind = get_pointer_type(Ptr, *C);
195196
switch (kind) {
196197
case usm::alloc::host:
197-
return "host";
198+
return DPCTLSyclUSMType::DPCTL_USM_HOST;
198199
case usm::alloc::device:
199-
return "device";
200+
return DPCTLSyclUSMType::DPCTL_USM_DEVICE;
200201
case usm::alloc::shared:
201-
return "shared";
202+
return DPCTLSyclUSMType::DPCTL_USM_SHARED;
202203
default:
203-
return "unknown";
204+
return DPCTLSyclUSMType::DPCTL_USM_UNKNOWN;
204205
}
205206
}
206207

libsyclinterface/tests/test_sycl_usm_interface.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ void common_test_body(size_t nbytes,
5050
auto Ctx = DPCTLQueue_GetContext(Q);
5151

5252
auto kind = DPCTLUSM_GetPointerType(Ptr, Ctx);
53-
EXPECT_TRUE(0 == std::strncmp(kind, expected, 4));
53+
EXPECT_TRUE(kind == expected);
5454

5555
auto Dev = DPCTLUSM_GetPointerDevice(Ptr, Ctx);
5656
auto QueueDev = DPCTLQueue_GetDevice(Q);
@@ -100,7 +100,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocShared)
100100
const size_t nbytes = SIZE;
101101
auto Ptr = DPCTLmalloc_shared(nbytes, Q);
102102
EXPECT_TRUE(bool(Ptr));
103-
common_test_body(nbytes, Ptr, Q, "shared");
103+
common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::shared);
104104
DPCTLfree_with_queue(Ptr, Q);
105105
DPCTLQueue_Delete(Q);
106106
}
@@ -112,7 +112,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocDevice)
112112
const size_t nbytes = SIZE;
113113
auto Ptr = DPCTLmalloc_device(nbytes, Q);
114114
EXPECT_TRUE(bool(Ptr));
115-
common_test_body(nbytes, Ptr, Q, "device");
115+
common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::device);
116116
DPCTLfree_with_queue(Ptr, Q);
117117
DPCTLQueue_Delete(Q);
118118
}
@@ -124,7 +124,7 @@ TEST_F(TestDPCTLSyclUSMInterface, MallocHost)
124124
const size_t nbytes = SIZE;
125125
auto Ptr = DPCTLmalloc_host(nbytes, Q);
126126
EXPECT_TRUE(bool(Ptr));
127-
common_test_body(nbytes, Ptr, Q, "host");
127+
common_test_body(nbytes, Ptr, Q, DPCTLSyclUSMType::host);
128128
DPCTLfree_with_queue(Ptr, Q);
129129
DPCTLQueue_Delete(Q);
130130
}

0 commit comments

Comments
 (0)