From 54c61a4574ec6fceeafbd39ee044b934ac777abc Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 27 May 2021 09:03:16 -0500 Subject: [PATCH 1/4] In CreateSubDevicesEqually and ByCounts check that counts by positive Also change error handling in CreateSubDevicesByCounts. --- .../source/dpctl_sycl_device_interface.cpp | 26 ++++++++++++------- .../tests/test_sycl_device_subdevices.cpp | 8 ++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/dpctl-capi/source/dpctl_sycl_device_interface.cpp b/dpctl-capi/source/dpctl_sycl_device_interface.cpp index d6c1af390d..8077a25907 100644 --- a/dpctl-capi/source/dpctl_sycl_device_interface.cpp +++ b/dpctl-capi/source/dpctl_sycl_device_interface.cpp @@ -29,6 +29,7 @@ #include "Support/CBindingWrapping.h" #include "dpctl_sycl_device_manager.h" #include /* SYCL headers */ +#include #include using namespace cl::sycl; @@ -577,8 +578,8 @@ DPCTLDevice_CreateSubDevicesEqually(__dpctl_keep const DPCTLSyclDeviceRef DRef, size_t count) { vector_class *Devices = nullptr; - auto D = unwrap(DRef); - if (D) { + if (DRef && count) { + auto D = unwrap(DRef); try { auto subDevices = D->create_sub_devices< info::partition_property::partition_equally>(count); @@ -612,11 +613,22 @@ DPCTLDevice_CreateSubDevicesByCounts(__dpctl_keep const DPCTLSyclDeviceRef DRef, vector_class *Devices = nullptr; std::vector vcounts; vcounts.assign(counts, counts + ncounts); - auto D = unwrap(DRef); - if (D) { + size_t min_elem = *std::min_element(vcounts.begin(), vcounts.end()); + if (DRef && min_elem) { + auto D = unwrap(DRef); + vector_class::type> subDevices; try { - auto subDevices = D->create_sub_devices< + subDevices = D->create_sub_devices< info::partition_property::partition_by_counts>(vcounts); + } catch (feature_not_supported const &fnse) { + std::cerr << fnse.what() << '\n'; + return nullptr; + } catch (runtime_error const &re) { + // \todo log error + std::cerr << re.what() << '\n'; + return nullptr; + } + try { Devices = new vector_class(); for (const auto &sd : subDevices) { Devices->emplace_back(wrap(new device(sd))); @@ -625,10 +637,6 @@ DPCTLDevice_CreateSubDevicesByCounts(__dpctl_keep const DPCTLSyclDeviceRef DRef, delete Devices; std::cerr << ba.what() << '\n'; return nullptr; - } catch (feature_not_supported const &fnse) { - delete Devices; - std::cerr << fnse.what() << '\n'; - return nullptr; } catch (runtime_error const &re) { delete Devices; // \todo log error diff --git a/dpctl-capi/tests/test_sycl_device_subdevices.cpp b/dpctl-capi/tests/test_sycl_device_subdevices.cpp index 444f8f10dd..1979826bfd 100644 --- a/dpctl-capi/tests/test_sycl_device_subdevices.cpp +++ b/dpctl-capi/tests/test_sycl_device_subdevices.cpp @@ -92,6 +92,9 @@ TEST_P(TestDPCTLSyclDeviceInterface, ChkCreateSubDevicesEqually) EXPECT_NO_FATAL_FAILURE(DPCTLDevice_Delete(pDRef)); EXPECT_NO_FATAL_FAILURE(DPCTLDeviceVector_Delete(DVRef)); } + EXPECT_NO_FATAL_FAILURE( + DVRef = DPCTLDevice_CreateSubDevicesEqually(DRef, 0)); + EXPECT_TRUE(DVRef == nullptr); } } @@ -114,7 +117,12 @@ TEST_P(TestDPCTLSyclDeviceInterface, ChkCreateSubDevicesByCounts) if (DVRef) { EXPECT_TRUE(DPCTLDeviceVector_Size(DVRef) > 0); EXPECT_NO_FATAL_FAILURE(DPCTLDeviceVector_Delete(DVRef)); + DVRef = nullptr; } + counts[n - 1] = 0; + EXPECT_NO_FATAL_FAILURE( + DVRef = DPCTLDevice_CreateSubDevicesByCounts(DRef, counts, n)); + EXPECT_TRUE(DVRef == nullptr); } } From f563d87530b2bf26ffdff2bce1008df3b50d6e93 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 27 May 2021 09:04:29 -0500 Subject: [PATCH 2/4] SyclDevice.create_sub_devices checks partition argument better Use of zero EU counts now raises and error. --- dpctl/_sycl_device.pyx | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/dpctl/_sycl_device.pyx b/dpctl/_sycl_device.pyx index 90882c8bfd..8be701c20e 100644 --- a/dpctl/_sycl_device.pyx +++ b/dpctl/_sycl_device.pyx @@ -706,7 +706,12 @@ cdef class SyclDevice(_SyclDevice): the sub-devices. """ cdef DPCTLDeviceVectorRef DVRef = NULL - DVRef = DPCTLDevice_CreateSubDevicesEqually(self._device_ref, count) + if count > 0: + DVRef = DPCTLDevice_CreateSubDevicesEqually(self._device_ref, count) + else: + raise ValueError( + "Creating sub-devices with zero compute units is requested" + ) if DVRef is NULL: raise SubDeviceCreationError("Sub-devices were not created.") return _get_devices(DVRef) @@ -720,6 +725,7 @@ cdef class SyclDevice(_SyclDevice): """ cdef int ncounts = len(counts) cdef size_t *counts_buff = NULL + cdef size_t min_count = 1 cdef DPCTLDeviceVectorRef DVRef = NULL cdef int i @@ -734,10 +740,17 @@ cdef class SyclDevice(_SyclDevice): ) for i in range(ncounts): counts_buff[i] = counts[i] - DVRef = DPCTLDevice_CreateSubDevicesByCounts( - self._device_ref, counts_buff, ncounts - ) + if counts_buff[i] == 0: + min_count = 0 + if min_count: + DVRef = DPCTLDevice_CreateSubDevicesByCounts( + self._device_ref, counts_buff, ncounts + ) free(counts_buff) + if min_count == 0: + raise ValueError( + "Targeted sub-device execution units must positive" + ) if DVRef is NULL: raise SubDeviceCreationError("Sub-devices were not created.") return _get_devices(DVRef) From 9aceb2de3b69e98ff702f5a7e5625b472a203b18 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 27 May 2021 09:40:31 -0500 Subject: [PATCH 3/4] Added tests that handling of partitions with zero EU counts works as designed --- dpctl/tests/test_sycl_device.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/dpctl/tests/test_sycl_device.py b/dpctl/tests/test_sycl_device.py index e55a5ee225..882bcebc6b 100644 --- a/dpctl/tests/test_sycl_device.py +++ b/dpctl/tests/test_sycl_device.py @@ -360,6 +360,13 @@ def check_create_sub_devices_equally(device): pytest.fail("create_sub_devices failed") +def check_create_sub_devices_equally_zeros(device): + try: + device.create_sub_devices(partition=0) + except TypeError: + pass + + def check_create_sub_devices_by_counts(device): try: n = device.max_compute_units / 2 @@ -372,6 +379,13 @@ def check_create_sub_devices_by_counts(device): pytest.fail("create_sub_devices failed") +def check_create_sub_devices_by_counts_zeros(device): + try: + device.create_sub_devices(partition=(0, 1)) + except TypeError: + pass + + def check_create_sub_devices_by_affinity_not_applicable(device): try: device.create_sub_devices(partition="not_applicable") From 5d23a0a8d2f5c1fc29f7eb310542dacec53b9379 Mon Sep 17 00:00:00 2001 From: Oleksandr Pavlyk Date: Thu, 27 May 2021 16:17:13 -0500 Subject: [PATCH 4/4] improved error handling of zero count --- .../source/dpctl_sycl_device_interface.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/dpctl-capi/source/dpctl_sycl_device_interface.cpp b/dpctl-capi/source/dpctl_sycl_device_interface.cpp index 8077a25907..80a0f983e0 100644 --- a/dpctl-capi/source/dpctl_sycl_device_interface.cpp +++ b/dpctl-capi/source/dpctl_sycl_device_interface.cpp @@ -578,7 +578,12 @@ DPCTLDevice_CreateSubDevicesEqually(__dpctl_keep const DPCTLSyclDeviceRef DRef, size_t count) { vector_class *Devices = nullptr; - if (DRef && count) { + if (DRef) { + if (count == 0) { + std::cerr << "Can not create sub-devices with zero compute units" + << '\n'; + return nullptr; + } auto D = unwrap(DRef); try { auto subDevices = D->create_sub_devices< @@ -611,10 +616,15 @@ DPCTLDevice_CreateSubDevicesByCounts(__dpctl_keep const DPCTLSyclDeviceRef DRef, size_t ncounts) { vector_class *Devices = nullptr; - std::vector vcounts; + std::vector vcounts(ncounts); vcounts.assign(counts, counts + ncounts); size_t min_elem = *std::min_element(vcounts.begin(), vcounts.end()); - if (DRef && min_elem) { + if (min_elem == 0) { + std::cerr << "Can not create sub-devices with zero compute units" + << '\n'; + return nullptr; + } + if (DRef) { auto D = unwrap(DRef); vector_class::type> subDevices; try {