Skip to content

Commit ba8e786

Browse files
[Unittests] Fix device info query for shared USM (#13801)
The unittests have a default mock implementation of device info queries which returns success for any query made. However, for cases where the query does not have a case in the implementation, nothing is done to the return values. For support queries this may mean that one call to the query returns true and another returns false. This has been seen for shared USM allocation support, which caused flaky failures when unittests would call malloc_shared. This commit: * Changes the default implementation of the device info query to return true for shared USM allocation support, like it already does for host and device variants. * Changes the default implementation of the device info query to fill the return value with 0 and set the return type size to 1 when the info query is unrecognized. This means unittests should always use `redefineAfter` for the device info query. * Fixes tests that assumes device info queries may be device-dependent in unittests. Signed-off-by: Larsen, Steffen <[email protected]>
1 parent 0cc31ce commit ba8e786

File tree

3 files changed

+31
-18
lines changed

3 files changed

+31
-18
lines changed

sycl/unittests/Extensions/CommandGraph/InOrderQueue.cpp

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -419,9 +419,12 @@ TEST_F(CommandGraphTest, InOrderQueueMemsetAndGraph) {
419419
experimental::command_graph<experimental::graph_state::modifiable>
420420
InOrderGraph{InOrderQueue.get_context(), InOrderQueue.get_device()};
421421

422-
// Check if device has usm shared allocation.
423-
if (!InOrderQueue.get_device().has(sycl::aspect::usm_shared_allocations))
424-
return;
422+
// The mock plugin should return true for shared USM allocation support by
423+
// default. If this fails it means this test needs to redefine the device info
424+
// query.
425+
ASSERT_TRUE(
426+
InOrderQueue.get_device().has(sycl::aspect::usm_shared_allocations));
427+
425428
size_t Size = 128;
426429
std::vector<int> TestDataHost(Size);
427430
int *TestData = sycl::malloc_shared<int>(Size, InOrderQueue);
@@ -494,9 +497,12 @@ TEST_F(CommandGraphTest, InOrderQueueMemcpyAndGraph) {
494497
experimental::command_graph<experimental::graph_state::modifiable>
495498
InOrderGraph{InOrderQueue.get_context(), InOrderQueue.get_device()};
496499

497-
// Check if device has usm shared allocation.
498-
if (!InOrderQueue.get_device().has(sycl::aspect::usm_shared_allocations))
499-
return;
500+
// The mock plugin should return true for shared USM allocation support by
501+
// default. If this fails it means this test needs to redefine the device info
502+
// query.
503+
ASSERT_TRUE(
504+
InOrderQueue.get_device().has(sycl::aspect::usm_shared_allocations));
505+
500506
size_t Size = 128;
501507
std::vector<int> TestDataHost(Size);
502508
int *TestData = sycl::malloc_shared<int>(Size, InOrderQueue);

sycl/unittests/helpers/PiMockPlugin.hpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ inline pi_result mock_piDeviceGetInfo(pi_device device,
204204
}
205205
case PI_DEVICE_INFO_USM_HOST_SUPPORT:
206206
case PI_DEVICE_INFO_USM_DEVICE_SUPPORT:
207+
case PI_DEVICE_INFO_USM_SINGLE_SHARED_SUPPORT:
207208
case PI_DEVICE_INFO_HOST_UNIFIED_MEMORY:
208209
case PI_DEVICE_INFO_AVAILABLE:
209210
case PI_DEVICE_INFO_LINKER_AVAILABLE:
@@ -238,9 +239,23 @@ inline pi_result mock_piDeviceGetInfo(pi_device device,
238239
}
239240
return PI_SUCCESS;
240241
}
241-
default:
242+
default: {
243+
// In the default case we fill the return value with 0's. This may not be
244+
// valid for all device queries, but it will mean a consistent return value
245+
// for the query.
246+
// Any tests that need special return values should either add behavior
247+
// the this function or use redefineAfter with a function that adds the
248+
// intended behavior.
249+
if (param_value && param_value_size != 0)
250+
std::memset(param_value, 0, param_value_size);
251+
// Likewise, if the device info query asks for the size of the return value
252+
// we tell it there is a single byte to avoid cases where the runtime tries
253+
// to allocate some random amount of memory for the return value.
254+
if (param_value_size_ret)
255+
*param_value_size_ret = 1;
242256
return PI_SUCCESS;
243257
}
258+
}
244259
}
245260

246261
inline pi_result mock_piDeviceRetain(pi_device device) { return PI_SUCCESS; }

sycl/unittests/kernel-and-program/DeviceInfo.cpp

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ class DeviceInfoTest : public ::testing::Test {
8383

8484
protected:
8585
void SetUp() override {
86-
Mock.redefineBefore<detail::PiApiKind::piDeviceGetInfo>(
86+
Mock.redefineAfter<detail::PiApiKind::piDeviceGetInfo>(
8787
redefinedDeviceGetInfo);
8888
}
8989

@@ -131,11 +131,7 @@ TEST_F(DeviceInfoTest, GetDeviceUUID) {
131131

132132
device Dev = Ctx.get_devices()[0];
133133

134-
if (!Dev.has(aspect::ext_intel_device_info_uuid)) {
135-
std::clog
136-
<< "This test is only for the devices with UUID extension support.\n";
137-
return;
138-
}
134+
EXPECT_TRUE(Dev.has(aspect::ext_intel_device_info_uuid));
139135

140136
auto UUID = Dev.get_info<ext::intel::info::device::uuid>();
141137

@@ -154,11 +150,7 @@ TEST_F(DeviceInfoTest, GetDeviceFreeMemory) {
154150

155151
device Dev = Ctx.get_devices()[0];
156152

157-
if (!Dev.has(aspect::ext_intel_free_memory)) {
158-
std::clog << "This test is only for the devices with "
159-
"ext_intel_free_memory extension support.\n";
160-
return;
161-
}
153+
EXPECT_TRUE(Dev.has(aspect::ext_intel_free_memory));
162154

163155
auto FreeMemory = Dev.get_info<ext::intel::info::device::free_memory>();
164156

0 commit comments

Comments
 (0)