From ea7751c67b7d0a255ec1e853c9328692f9325aa7 Mon Sep 17 00:00:00 2001 From: Alexey Sachkov Date: Fri, 5 Jul 2024 10:52:07 +0200 Subject: [PATCH] [SYCL] Fix reporting duplicate composite devices (#14442) This patch intended to be an improvement for composite device implementation code coverage, but it turned out that it also helped to find a bug in the implementaiton. --- sycl/source/platform.cpp | 3 +- sycl/unittests/Extensions/CompositeDevice.cpp | 71 ++++++++++++++++--- 2 files changed, 65 insertions(+), 9 deletions(-) diff --git a/sycl/source/platform.cpp b/sycl/source/platform.cpp index 1b717c6093abc..cedf11ed23d58 100644 --- a/sycl/source/platform.cpp +++ b/sycl/source/platform.cpp @@ -118,7 +118,8 @@ std::vector platform::ext_oneapi_get_composite_devices() const { auto Composite = Dev.get_info< sycl::ext::oneapi::experimental::info::device::composite_device>(); - if (std::find(Result.begin(), Result.end(), Composite) == Result.end()) + if (std::find(Composites.begin(), Composites.end(), Composite) == + Composites.end()) Composites.push_back(Composite); } for (const auto &Composite : Composites) { diff --git a/sycl/unittests/Extensions/CompositeDevice.cpp b/sycl/unittests/Extensions/CompositeDevice.cpp index 93073f78319db..687e18df79597 100644 --- a/sycl/unittests/Extensions/CompositeDevice.cpp +++ b/sycl/unittests/Extensions/CompositeDevice.cpp @@ -7,20 +7,30 @@ #include namespace { -const auto COMPOSITE_DEVICE = reinterpret_cast(1u); +const auto COMPOSITE_DEVICE_0 = reinterpret_cast(1u); const auto COMPONENT_DEVICE_A = reinterpret_cast(2u); const auto COMPONENT_DEVICE_B = reinterpret_cast(3u); +// We do not report COMPONENT_DEVICE_D through mocked piDevicesGet to emulate +// that it is not available to ensure that COMPOSITE_DEVICE_1 is not returned +// through platform::ext_oneapi_get_composite_devices and +// sycl:ext::oneapi::experimental::get_composite_devices APIs +const auto COMPOSITE_DEVICE_1 = reinterpret_cast(4u); +const auto COMPONENT_DEVICE_C = reinterpret_cast(5u); +const auto COMPONENT_DEVICE_D = reinterpret_cast(6u); + pi_result redefine_piDevicesGet(pi_platform platform, pi_device_type, pi_uint32 num_entries, pi_device *devices, pi_uint32 *num_devices) { if (num_devices) - *num_devices = 2; + *num_devices = 3; if (devices) { if (num_entries > 0) devices[0] = COMPONENT_DEVICE_A; if (num_entries > 1) devices[1] = COMPONENT_DEVICE_B; + if (num_entries > 2) + devices[2] = COMPONENT_DEVICE_C; } return PI_SUCCESS; } @@ -34,7 +44,9 @@ pi_result after_piDeviceGetInfo(pi_device device, pi_device_info param_name, *param_value_size_ret = sizeof(pi_device); if (param_value) { if (device == COMPONENT_DEVICE_A || device == COMPONENT_DEVICE_B) { - *static_cast(param_value) = COMPOSITE_DEVICE; + *static_cast(param_value) = COMPOSITE_DEVICE_0; + } else if (device == COMPONENT_DEVICE_C || device == COMPONENT_DEVICE_D) { + *static_cast(param_value) = COMPOSITE_DEVICE_1; } else *static_cast(param_value) = nullptr; } @@ -42,7 +54,7 @@ pi_result after_piDeviceGetInfo(pi_device device, pi_device_info param_name, return PI_SUCCESS; case PI_EXT_ONEAPI_DEVICE_INFO_COMPONENT_DEVICES: - if (device == COMPOSITE_DEVICE) { + if (device == COMPOSITE_DEVICE_0) { if (param_value_size_ret) *param_value_size_ret = 2 * sizeof(pi_device); if (param_value) { @@ -51,7 +63,15 @@ pi_result after_piDeviceGetInfo(pi_device device, pi_device_info param_name, if (param_value_size >= 2 * sizeof(pi_device)) static_cast(param_value)[1] = COMPONENT_DEVICE_B; } - + } else if (device == COMPOSITE_DEVICE_1) { + if (param_value_size_ret) + *param_value_size_ret = 2 * sizeof(pi_device); + if (param_value) { + if (param_value_size >= sizeof(pi_device)) + static_cast(param_value)[0] = COMPONENT_DEVICE_C; + if (param_value_size >= 2 * sizeof(pi_device)) + static_cast(param_value)[1] = COMPONENT_DEVICE_D; + } } else { if (param_value_size_ret) *param_value_size_ret = 0; @@ -110,6 +130,41 @@ pi_result after_piContextCreate(const pi_context_properties *, } // namespace +TEST(CompositeDeviceTest, PlatformExtOneAPIGetCompositeDevices) { + sycl::unittest::PiMock Mock; + Mock.redefine(redefine_piDevicesGet); + Mock.redefineAfter( + after_piDeviceGetInfo); + + sycl::platform Plt = Mock.getPlatform(); + + std::vector Composites = Plt.ext_oneapi_get_composite_devices(); + // We don't expect to see COMPOSITE_DEVICE_1 here, because one of its + // components (COMPONENT_DEVICE_D) is not available. + ASSERT_EQ(Composites.size(), 1u); + ASSERT_EQ(sycl::bit_cast( + sycl::get_native(Composites.front())), + COMPOSITE_DEVICE_0); +} + +TEST(CompositeDeviceTest, SYCLExtOneAPIExperimentalGetCompositeDevices) { + sycl::unittest::PiMock Mock; + Mock.redefine(redefine_piDevicesGet); + Mock.redefineAfter( + after_piDeviceGetInfo); + + sycl::platform Plt = Mock.getPlatform(); + + std::vector Composites = + sycl::ext::oneapi::experimental::get_composite_devices(); + // We don't expect to see COMPOSITE_DEVICE_1 here, because one of its + // components (COMPONENT_DEVICE_D) is not available. + ASSERT_EQ(Composites.size(), 1u); + ASSERT_EQ(sycl::bit_cast( + sycl::get_native(Composites.front())), + COMPOSITE_DEVICE_0); +} + TEST(CompositeDeviceTest, DescendentDeviceSupportInContext) { sycl::unittest::PiMock Mock; Mock.redefine(redefine_piDevicesGet); @@ -133,9 +188,9 @@ TEST(CompositeDeviceTest, DescendentDeviceSupportInContext) { // created for a composite device, we expect them to be implicitly added to // the context under the hood. ASSERT_EQ(DevicesUsedInContextCreation.size(), 3u); - ASSERT_TRUE(std::any_of(DevicesUsedInContextCreation.begin(), - DevicesUsedInContextCreation.end(), - [=](pi_device D) { return D == COMPOSITE_DEVICE; })); + ASSERT_TRUE(std::any_of( + DevicesUsedInContextCreation.begin(), DevicesUsedInContextCreation.end(), + [=](pi_device D) { return D == COMPOSITE_DEVICE_0; })); ASSERT_TRUE(std::any_of( DevicesUsedInContextCreation.begin(), DevicesUsedInContextCreation.end(), [=](pi_device D) { return D == COMPONENT_DEVICE_A; }));