Merge pull request #2675 from martygrant/martin/enqueue-cts-spec-gap

Improvements to align CTS and Spec for Enqueue

Author: martygrant

include/ur_api.h

@@ -8295,8 +8295,6 @@ typedef enum ur_usm_migration_flag_t {
 ///        address space and return a pointer to the mapped region
 ///
 /// @details
-///     - Input parameter blockingMap indicates if the map is blocking or
-///       non-blocking.
 ///     - Currently, no direct support in Level Zero. Implemented as a shared
 ///       allocation followed by copying on discrete GPU
 ///     - TODO: add a driver function in Level Zero?
@@ -8569,7 +8567,8 @@ UR_APIEXPORT ur_result_t UR_APICALL urEnqueueUSMPrefetch(
 /// @details
 ///     - Not all memory advice hints may be supported for all devices or
 ///       allocation types. If a memory advice hint is not supported, it will be
-///       ignored.
+///       ignored. Some adapters may return ::UR_RESULT_ERROR_ADAPTER_SPECIFIC,
+///       more information can be retrieved by using urAdapterGetLastError.
 ///
 /// @returns
 ///     - ::UR_RESULT_SUCCESS

scripts/core/enqueue.yml

@@ -863,7 +863,6 @@ class: $xEnqueue
 name: MemBufferMap
 ordinal: "0"
 details:
-    - "Input parameter blockingMap indicates if the map is blocking or non-blocking."
     - "Currently, no direct support in Level Zero. Implemented as a shared allocation followed by copying on discrete GPU"
     - "TODO: add a driver function in Level Zero?"
 analogue:
@@ -1180,7 +1179,7 @@ class: $xEnqueue
 name: USMAdvise
 ordinal: "0"
 details:
-    - "Not all memory advice hints may be supported for all devices or allocation types. If a memory advice hint is not supported, it will be ignored."
+    - "Not all memory advice hints may be supported for all devices or allocation types. If a memory advice hint is not supported, it will be ignored. Some adapters may return $X_RESULT_ERROR_ADAPTER_SPECIFIC, more information can be retrieved by using urAdapterGetLastError."
 params:
     - type: $x_queue_handle_t
       name: hQueue

source/loader/ur_libapi.cpp

@@ -5951,8 +5951,6 @@ ur_result_t UR_APICALL urEnqueueMemImageCopy(
 ///        address space and return a pointer to the mapped region
 ///
 /// @details
-///     - Input parameter blockingMap indicates if the map is blocking or
-///       non-blocking.
 ///     - Currently, no direct support in Level Zero. Implemented as a shared
 ///       allocation followed by copying on discrete GPU
 ///     - TODO: add a driver function in Level Zero?
@@ -6272,7 +6270,8 @@ ur_result_t UR_APICALL urEnqueueUSMPrefetch(
 /// @details
 ///     - Not all memory advice hints may be supported for all devices or
 ///       allocation types. If a memory advice hint is not supported, it will be
-///       ignored.
+///       ignored. Some adapters may return ::UR_RESULT_ERROR_ADAPTER_SPECIFIC,
+///       more information can be retrieved by using urAdapterGetLastError.
 ///
 /// @returns
 ///     - ::UR_RESULT_SUCCESS

source/ur_api.cpp

@@ -5218,8 +5218,6 @@ ur_result_t UR_APICALL urEnqueueMemImageCopy(
 ///        address space and return a pointer to the mapped region
 ///
 /// @details
-///     - Input parameter blockingMap indicates if the map is blocking or
-///       non-blocking.
 ///     - Currently, no direct support in Level Zero. Implemented as a shared
 ///       allocation followed by copying on discrete GPU
 ///     - TODO: add a driver function in Level Zero?
@@ -5507,7 +5505,8 @@ ur_result_t UR_APICALL urEnqueueUSMPrefetch(
 /// @details
 ///     - Not all memory advice hints may be supported for all devices or
 ///       allocation types. If a memory advice hint is not supported, it will be
-///       ignored.
+///       ignored. Some adapters may return ::UR_RESULT_ERROR_ADAPTER_SPECIFIC,
+///       more information can be retrieved by using urAdapterGetLastError.
 ///
 /// @returns
 ///     - ::UR_RESULT_SUCCESS

test/conformance/enqueue/urEnqueueDeviceGlobalVariableRead.cpp

@@ -5,10 +5,16 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #include <uur/fixtures.h>
 
-using urEnqueueDeviceGetGlobalVariableReadTest = uur::urGlobalVariableTest;
-UUR_INSTANTIATE_DEVICE_TEST_SUITE(urEnqueueDeviceGetGlobalVariableReadTest);
+using urEnqueueDeviceGetGlobalVariableReadWithParamTest =
+    uur::urGlobalVariableWithParamTest<uur::BoolTestParam>;
+
+UUR_DEVICE_TEST_SUITE_WITH_PARAM(
+    urEnqueueDeviceGetGlobalVariableReadWithParamTest,
+    testing::ValuesIn(uur::BoolTestParam::makeBoolParam("Blocking")),
+    uur::deviceTestWithParamPrinter<uur::BoolTestParam>);
 
-TEST_P(urEnqueueDeviceGetGlobalVariableReadTest, Success) {
+TEST_P(urEnqueueDeviceGetGlobalVariableReadWithParamTest, Success) {
+  bool is_blocking = getParam().value;
 
   ASSERT_SUCCESS(urEnqueueDeviceGlobalVariableWrite(
       queue, program, global_var.name.c_str(), true, sizeof(global_var.value),
@@ -26,13 +32,20 @@ TEST_P(urEnqueueDeviceGetGlobalVariableReadTest, Success) {
 
   // read global var back to host
   ASSERT_SUCCESS(urEnqueueDeviceGlobalVariableRead(
-      queue, program, global_var.name.c_str(), true, sizeof(global_var.value),
-      0, &global_var.value, 0, nullptr, nullptr));
+      queue, program, global_var.name.c_str(), is_blocking,
+      sizeof(global_var.value), 0, &global_var.value, 0, nullptr, nullptr));
+
+  if (!is_blocking) {
+    ASSERT_SUCCESS(urQueueFinish(queue));
+  }
 
   // kernel should increment value
   ASSERT_EQ(global_var.value, 1);
 }
 
+using urEnqueueDeviceGetGlobalVariableReadTest = uur::urGlobalVariableTest;
+UUR_INSTANTIATE_DEVICE_TEST_SUITE(urEnqueueDeviceGetGlobalVariableReadTest);
+
 TEST_P(urEnqueueDeviceGetGlobalVariableReadTest, InvalidNullHandleQueue) {
   ASSERT_EQ_RESULT(urEnqueueDeviceGlobalVariableRead(
                        nullptr, program, global_var.name.c_str(), true,

test/conformance/enqueue/urEnqueueDeviceGlobalVariableWrite.cpp

@@ -5,6 +5,46 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 #include <uur/fixtures.h>
 
+using urEnqueueDeviceGetGlobalVariableWriteWithParamTest =
+    uur::urGlobalVariableWithParamTest<uur::BoolTestParam>;
+
+UUR_DEVICE_TEST_SUITE_WITH_PARAM(
+    urEnqueueDeviceGetGlobalVariableWriteWithParamTest,
+    testing::ValuesIn(uur::BoolTestParam::makeBoolParam("Blocking")),
+    uur::deviceTestWithParamPrinter<uur::BoolTestParam>);
+
+TEST_P(urEnqueueDeviceGetGlobalVariableWriteWithParamTest, Success) {
+  bool is_blocking = getParam().value;
+  global_var.value = 42;
+
+  ASSERT_SUCCESS(urEnqueueDeviceGlobalVariableWrite(
+      queue, program, global_var.name.c_str(), is_blocking,
+      sizeof(global_var.value), 0, &global_var.value, 0, nullptr, nullptr));
+
+  if (!is_blocking) {
+    ASSERT_SUCCESS(urQueueFinish(queue));
+  }
+
+  size_t global_offset = 0;
+  size_t n_dimensions = 1;
+  size_t global_size = 1;
+
+  // execute the kernel
+  ASSERT_SUCCESS(urEnqueueKernelLaunch(queue, kernel, n_dimensions,
+                                       &global_offset, &global_size, nullptr, 0,
+                                       nullptr, nullptr));
+  ASSERT_SUCCESS(urQueueFinish(queue));
+
+  // read global var back to host
+  int return_value = 0;
+  ASSERT_SUCCESS(urEnqueueDeviceGlobalVariableRead(
+      queue, program, global_var.name.c_str(), true, sizeof(return_value), 0,
+      &return_value, 0, nullptr, nullptr));
+
+  // kernel should return global_var.value + 1
+  ASSERT_EQ(return_value, global_var.value + 1);
+}
+
 using urEnqueueDeviceGetGlobalVariableWriteTest = uur::urGlobalVariableTest;
 UUR_INSTANTIATE_DEVICE_TEST_SUITE(urEnqueueDeviceGetGlobalVariableWriteTest);
 

test/conformance/enqueue/urEnqueueKernelLaunch.cpp

@@ -85,6 +85,18 @@ TEST_P(urEnqueueKernelLaunchTest, InvalidNullHandleQueue) {
                    UR_RESULT_ERROR_INVALID_NULL_HANDLE);
 }
 
+TEST_P(urEnqueueKernelLaunchTest, InvalidNullPointer) {
+  ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, kernel, n_dimensions, nullptr,
+                                         &global_size, nullptr, 0, nullptr,
+                                         nullptr),
+                   UR_RESULT_ERROR_INVALID_NULL_POINTER);
+
+  ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, kernel, n_dimensions,
+                                         &global_offset, nullptr, nullptr, 0,
+                                         nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_NULL_POINTER);
+}
+
 TEST_P(urEnqueueKernelLaunchTest, InvalidNullHandleKernel) {
   ASSERT_EQ_RESULT(urEnqueueKernelLaunch(queue, nullptr, n_dimensions,
                                          &global_offset, &global_size, nullptr,

test/conformance/enqueue/urEnqueueMemBufferCopyRect.cpp

@@ -177,13 +177,17 @@ struct urEnqueueMemBufferCopyRectTest : uur::urQueueTest {
   ur_mem_handle_t src_buffer = nullptr;
   ur_mem_handle_t dst_buffer = nullptr;
   std::vector<uint32_t> input;
+  ur_rect_region_t src_region{size, 1, 1};
+  ur_rect_offset_t src_origin{0, 0, 0};
+  ur_rect_offset_t dst_origin{0, 0, 0};
+  size_t src_row_pitch = size;
+  size_t src_slice_pitch = size;
+  const size_t dst_row_pitch = size;
+  size_t dst_slice_pitch = size;
 };
 UUR_INSTANTIATE_DEVICE_TEST_SUITE(urEnqueueMemBufferCopyRectTest);
 
 TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleQueue) {
-  ur_rect_region_t src_region{size, 1, 1};
-  ur_rect_offset_t src_origin{0, 0, 0};
-  ur_rect_offset_t dst_origin{0, 0, 0};
   ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE,
                    urEnqueueMemBufferCopyRect(nullptr, src_buffer, dst_buffer,
                                               src_origin, dst_origin,
@@ -192,9 +196,6 @@ TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleQueue) {
 }
 
 TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleBufferSrc) {
-  ur_rect_region_t src_region{size, 1, 1};
-  ur_rect_offset_t src_origin{0, 0, 0};
-  ur_rect_offset_t dst_origin{0, 0, 0};
   ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE,
                    urEnqueueMemBufferCopyRect(queue, nullptr, dst_buffer,
                                               src_origin, dst_origin,
@@ -203,9 +204,6 @@ TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleBufferSrc) {
 }
 
 TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleBufferDst) {
-  ur_rect_region_t src_region{size, 1, 1};
-  ur_rect_offset_t src_origin{0, 0, 0};
-  ur_rect_offset_t dst_origin{0, 0, 0};
   ASSERT_EQ_RESULT(UR_RESULT_ERROR_INVALID_NULL_HANDLE,
                    urEnqueueMemBufferCopyRect(queue, src_buffer, nullptr,
                                               src_origin, dst_origin,
@@ -216,9 +214,6 @@ TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullHandleBufferDst) {
 TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullPtrEventWaitList) {
   UUR_KNOWN_FAILURE_ON(uur::NativeCPU{});
 
-  ur_rect_region_t src_region{size, 1, 1};
-  ur_rect_offset_t src_origin{0, 0, 0};
-  ur_rect_offset_t dst_origin{0, 0, 0};
   ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
                        queue, src_buffer, dst_buffer, src_origin, dst_origin,
                        src_region, size, size, size, size, 1, nullptr, nullptr),
@@ -242,6 +237,79 @@ TEST_P(urEnqueueMemBufferCopyRectTest, InvalidNullPtrEventWaitList) {
 
   ASSERT_SUCCESS(urEventRelease(validEvent));
 }
+TEST_P(urEnqueueMemBufferCopyRectTest, InvalidSize) {
+  UUR_KNOWN_FAILURE_ON(uur::NativeCPU{});
+
+  // region.width == 0 || region.height == 0 || region.width == 0
+  src_region.width = 0;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // srcRowPitch != 0 && srcRowPitch < region.width
+  src_region.width = src_row_pitch + 1;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // dstRowPitch != 0 && dstRowPitch < region.width
+  src_region.width = dst_row_pitch + 1;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // srcSlicePitch != 0 && srcSlicePitch < region.height * (srcRowPitch != 0 ?
+  // srcRowPitch : region.width)
+  src_region.width = size;
+  src_row_pitch = 16;
+  src_slice_pitch = (src_region.height * src_row_pitch) - 1;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // srcSlicePitch != 0 && srcSlicePitch % (srcRowPitch != 0 ? srcRowPitch :
+  // region.width) != 0
+  src_slice_pitch = size + 1;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // dstSlicePitch != 0 && dstSlicePitch < region.height * (dstRowPitch != 0 ?
+  // dstRowPitch : region.width)
+  src_slice_pitch = size;
+  dst_slice_pitch = (src_region.height * src_slice_pitch) - 1;
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, src_row_pitch, src_slice_pitch,
+                       dst_row_pitch, dst_slice_pitch, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // If the combination of srcOrigin, region, srcRowPitch, and srcSlicePitch
+  // results in an out-of-bounds access.
+  src_origin = {std::numeric_limits<uint64_t>::max(), 1, 1};
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, size, size, size, size, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+
+  // If the combination of dstOrigin, region, dstRowPitch, and dstSlicePitch
+  // results in an out-of-bounds access.
+  dst_origin = {std::numeric_limits<uint64_t>::max(), 1, 1};
+  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
+                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
+                       src_region, size, size, size, size, 0, nullptr, nullptr),
+                   UR_RESULT_ERROR_INVALID_SIZE);
+}
 
 using urEnqueueMemBufferCopyRectMultiDeviceTest =
     uur::urMultiDeviceMemBufferQueueTest;
@@ -281,17 +349,3 @@ TEST_P(urEnqueueMemBufferCopyRectMultiDeviceTest, CopyRectReadDifferentQueues) {
 
   EXPECT_SUCCESS(urMemRelease(dst_buffer));
 }
-
-TEST_P(urEnqueueMemBufferCopyRectTest, InvalidSize) {
-  UUR_KNOWN_FAILURE_ON(uur::NativeCPU{});
-
-  // out-of-bounds access with potential overflow
-  ur_rect_region_t src_region{size, 1, 1};
-  ur_rect_offset_t src_origin{std::numeric_limits<uint64_t>::max(), 1, 1};
-  ur_rect_offset_t dst_origin{0, 0, 0};
-
-  ASSERT_EQ_RESULT(urEnqueueMemBufferCopyRect(
-                       queue, src_buffer, dst_buffer, src_origin, dst_origin,
-                       src_region, size, size, size, size, 0, nullptr, nullptr),
-                   UR_RESULT_ERROR_INVALID_SIZE);
-}