Skip to content

Commit

Permalink
[SYCL] Fix reporting duplicate composite devices (#14442)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AlexeySachkov committed Jul 5, 2024
1 parent 3cc71b0 commit ea7751c
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 9 deletions.
3 changes: 2 additions & 1 deletion sycl/source/platform.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ std::vector<device> 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) {
Expand Down
71 changes: 63 additions & 8 deletions sycl/unittests/Extensions/CompositeDevice.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,30 @@
#include <algorithm>

namespace {
const auto COMPOSITE_DEVICE = reinterpret_cast<pi_device>(1u);
const auto COMPOSITE_DEVICE_0 = reinterpret_cast<pi_device>(1u);
const auto COMPONENT_DEVICE_A = reinterpret_cast<pi_device>(2u);
const auto COMPONENT_DEVICE_B = reinterpret_cast<pi_device>(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<pi_device>(4u);
const auto COMPONENT_DEVICE_C = reinterpret_cast<pi_device>(5u);
const auto COMPONENT_DEVICE_D = reinterpret_cast<pi_device>(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;
}
Expand All @@ -34,15 +44,17 @@ 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<pi_device *>(param_value) = COMPOSITE_DEVICE;
*static_cast<pi_device *>(param_value) = COMPOSITE_DEVICE_0;
} else if (device == COMPONENT_DEVICE_C || device == COMPONENT_DEVICE_D) {
*static_cast<pi_device *>(param_value) = COMPOSITE_DEVICE_1;
} else
*static_cast<pi_device *>(param_value) = nullptr;
}

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) {
Expand All @@ -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<pi_device *>(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<pi_device *>(param_value)[0] = COMPONENT_DEVICE_C;
if (param_value_size >= 2 * sizeof(pi_device))
static_cast<pi_device *>(param_value)[1] = COMPONENT_DEVICE_D;
}
} else {
if (param_value_size_ret)
*param_value_size_ret = 0;
Expand Down Expand Up @@ -110,6 +130,41 @@ pi_result after_piContextCreate(const pi_context_properties *,

} // namespace

TEST(CompositeDeviceTest, PlatformExtOneAPIGetCompositeDevices) {
sycl::unittest::PiMock Mock;
Mock.redefine<sycl::detail::PiApiKind::piDevicesGet>(redefine_piDevicesGet);
Mock.redefineAfter<sycl::detail::PiApiKind::piDeviceGetInfo>(
after_piDeviceGetInfo);

sycl::platform Plt = Mock.getPlatform();

std::vector<sycl::device> 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<pi_device>(
sycl::get_native<sycl::backend::opencl>(Composites.front())),
COMPOSITE_DEVICE_0);
}

TEST(CompositeDeviceTest, SYCLExtOneAPIExperimentalGetCompositeDevices) {
sycl::unittest::PiMock Mock;
Mock.redefine<sycl::detail::PiApiKind::piDevicesGet>(redefine_piDevicesGet);
Mock.redefineAfter<sycl::detail::PiApiKind::piDeviceGetInfo>(
after_piDeviceGetInfo);

sycl::platform Plt = Mock.getPlatform();

std::vector<sycl::device> 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<pi_device>(
sycl::get_native<sycl::backend::opencl>(Composites.front())),
COMPOSITE_DEVICE_0);
}

TEST(CompositeDeviceTest, DescendentDeviceSupportInContext) {
sycl::unittest::PiMock Mock;
Mock.redefine<sycl::detail::PiApiKind::piDevicesGet>(redefine_piDevicesGet);
Expand All @@ -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; }));
Expand Down

0 comments on commit ea7751c

Please sign in to comment.