Skip to content

Commit d88481b

Browse files
aarongreigaarongreig
authored
Merge pull request #2263 from aarongreig/aaron/clarifyNonBlockingFree
Explicitly specify USMFree as a blocking operation.
2 parents 72e80a4 + 974d099 commit d88481b

File tree

8 files changed

+101
-2
lines changed

8 files changed

+101
-2
lines changed

‎include/ur_api.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3666,6 +3666,11 @@ urUSMSharedAlloc(
36663666
///////////////////////////////////////////////////////////////////////////////
36673667
/// @brief Free the USM memory object
36683668
///
3669+
/// @details
3670+
/// - Note that implementations are required to wait for previously enqueued
3671+
/// commands that may be accessing `pMem` to finish before freeing the
3672+
/// memory.
3673+
///
36693674
/// @returns
36703675
/// - ::UR_RESULT_SUCCESS
36713676
/// - ::UR_RESULT_ERROR_UNINITIALIZED

‎scripts/core/usm.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,8 @@ desc: "Free the USM memory object"
364364
class: $xUSM
365365
name: Free
366366
ordinal: "0"
367+
details:
368+
- "Note that implementations are required to wait for previously enqueued commands that may be accessing `pMem` to finish before freeing the memory."
367369
params:
368370
- type: $x_context_handle_t
369371
name: hContext

‎source/loader/ur_libapi.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2404,6 +2404,11 @@ ur_result_t UR_APICALL urUSMSharedAlloc(
24042404
///////////////////////////////////////////////////////////////////////////////
24052405
/// @brief Free the USM memory object
24062406
///
2407+
/// @details
2408+
/// - Note that implementations are required to wait for previously enqueued
2409+
/// commands that may be accessing `pMem` to finish before freeing the
2410+
/// memory.
2411+
///
24072412
/// @returns
24082413
/// - ::UR_RESULT_SUCCESS
24092414
/// - ::UR_RESULT_ERROR_UNINITIALIZED

‎source/ur_api.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2080,6 +2080,11 @@ ur_result_t UR_APICALL urUSMSharedAlloc(
20802080
///////////////////////////////////////////////////////////////////////////////
20812081
/// @brief Free the USM memory object
20822082
///
2083+
/// @details
2084+
/// - Note that implementations are required to wait for previously enqueued
2085+
/// commands that may be accessing `pMem` to finish before freeing the
2086+
/// memory.
2087+
///
20832088
/// @returns
20842089
/// - ::UR_RESULT_SUCCESS
20852090
/// - ::UR_RESULT_ERROR_UNINITIALIZED

‎test/conformance/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ add_subdirectory(platform)
112112
add_subdirectory(device)
113113
add_subdirectory(context)
114114
add_subdirectory(memory)
115-
add_subdirectory(usm)
116115
add_subdirectory(event)
117116
add_subdirectory(queue)
118117
add_subdirectory(sampler)
@@ -129,6 +128,7 @@ set(TEST_SUBDIRECTORIES_DPCXX
129128
"exp_usm_p2p"
130129
"exp_launch_properties"
131130
"memory-migrate"
131+
"usm"
132132
)
133133

134134
if(UR_DPCXX)

‎test/conformance/usm/CMakeLists.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# See LICENSE.TXT
44
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
55

6-
add_conformance_test_with_devices_environment(usm
6+
add_conformance_test_with_kernels_environment(usm
77
urUSMDeviceAlloc.cpp
88
urUSMFree.cpp
99
urUSMGetMemAllocInfo.cpp

‎test/conformance/usm/urUSMFree.cpp‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,3 +95,84 @@ TEST_P(urUSMFreeTest, InvalidNullPtrMem) {
9595
ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_POINTER,
9696
urUSMFree(context, nullptr));
9797
}
98+
99+
// This goal of this test is to ensure urUSMFree blocks and waits for operations
100+
// accessing the given allocation to finish before actually freeing the memory.
101+
struct urUSMFreeDuringExecutionTest : uur::urKernelExecutionTest {
102+
void SetUp() {
103+
program_name = "fill_usm";
104+
UUR_RETURN_ON_FATAL_FAILURE(urKernelExecutionTest::SetUp());
105+
}
106+
107+
void *allocation = nullptr;
108+
size_t array_size = 256;
109+
size_t allocation_size = array_size * sizeof(uint32_t);
110+
uint32_t data = 42;
111+
size_t wg_offset = 0;
112+
};
113+
UUR_INSTANTIATE_KERNEL_TEST_SUITE_P(urUSMFreeDuringExecutionTest);
114+
115+
TEST_P(urUSMFreeDuringExecutionTest, SuccessHost) {
116+
ur_device_usm_access_capability_flags_t host_usm_flags = 0;
117+
ASSERT_SUCCESS(uur::GetDeviceUSMHostSupport(device, host_usm_flags));
118+
if (!(host_usm_flags & UR_DEVICE_USM_ACCESS_CAPABILITY_FLAG_ACCESS)) {
119+
GTEST_SKIP() << "Host USM is not supported.";
120+
}
121+
122+
ASSERT_SUCCESS(urUSMHostAlloc(context, nullptr, nullptr, allocation_size,
123+
&allocation));
124+
ASSERT_NE(allocation, nullptr);
125+
126+
EXPECT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
127+
EXPECT_SUCCESS(
128+
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
129+
EXPECT_SUCCESS(urEnqueueKernelLaunch(queue, kernel, 1, &wg_offset,
130+
&array_size, nullptr, 0, nullptr,
131+
nullptr));
132+
ASSERT_SUCCESS(urUSMFree(context, allocation));
133+
ASSERT_SUCCESS(urQueueFinish(queue));
134+
}
135+
136+
TEST_P(urUSMFreeDuringExecutionTest, SuccessDevice) {
137+
ur_device_usm_access_capability_flags_t device_usm_flags = 0;
138+
ASSERT_SUCCESS(uur::GetDeviceUSMDeviceSupport(device, device_usm_flags));
139+
if (!(device_usm_flags & UR_DEVICE_USM_ACCESS_CAPABILITY_FLAG_ACCESS)) {
140+
GTEST_SKIP() << "Device USM is not supported.";
141+
}
142+
143+
ASSERT_SUCCESS(urUSMDeviceAlloc(context, device, nullptr, nullptr,
144+
allocation_size, &allocation));
145+
ASSERT_NE(allocation, nullptr);
146+
147+
EXPECT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
148+
EXPECT_SUCCESS(
149+
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
150+
151+
EXPECT_SUCCESS(urEnqueueKernelLaunch(queue, kernel, 1, &wg_offset,
152+
&array_size, nullptr, 0, nullptr,
153+
nullptr));
154+
ASSERT_SUCCESS(urUSMFree(context, allocation));
155+
ASSERT_SUCCESS(urQueueFinish(queue));
156+
}
157+
158+
TEST_P(urUSMFreeDuringExecutionTest, SuccessShared) {
159+
ur_device_usm_access_capability_flags_t shared_usm_flags = 0;
160+
ASSERT_SUCCESS(
161+
uur::GetDeviceUSMSingleSharedSupport(device, shared_usm_flags));
162+
if (!(shared_usm_flags & UR_DEVICE_USM_ACCESS_CAPABILITY_FLAG_ACCESS)) {
163+
GTEST_SKIP() << "Shared USM is not supported.";
164+
}
165+
166+
ASSERT_SUCCESS(urUSMSharedAlloc(context, device, nullptr, nullptr,
167+
allocation_size, &allocation));
168+
ASSERT_NE(allocation, nullptr);
169+
170+
EXPECT_SUCCESS(urKernelSetArgPointer(kernel, 0, nullptr, allocation));
171+
EXPECT_SUCCESS(
172+
urKernelSetArgValue(kernel, 1, sizeof(data), nullptr, &data));
173+
EXPECT_SUCCESS(urEnqueueKernelLaunch(queue, kernel, 1, &wg_offset,
174+
&array_size, nullptr, 0, nullptr,
175+
nullptr));
176+
ASSERT_SUCCESS(urUSMFree(context, allocation));
177+
ASSERT_SUCCESS(urQueueFinish(queue));
178+
}

‎test/conformance/usm/usm_adapter_native_cpu.match‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ urUSMDeviceAllocTest.InvalidUSMSize/*__UsePoolDisabled
33
urUSMFreeTest.SuccessDeviceAlloc/*
44
urUSMFreeTest.SuccessHostAlloc/*
55
urUSMFreeTest.SuccessSharedAlloc/*
6+
urUSMFreeDuringExecutionTest.*
67
urUSMGetMemAllocInfoTest.Success/*__UR_USM_ALLOC_INFO_TYPE
78
urUSMGetMemAllocInfoTest.Success/*__UR_USM_ALLOC_INFO_BASE_PTR
89
urUSMGetMemAllocInfoTest.Success/*__UR_USM_ALLOC_INFO_SIZE

0 commit comments

Comments
 (0)