Require OpenCL simultaneous use (#17658)

To support the SYCL-Graph extension on an OpenCL backend, we currently
only require the presence of the `cl_khr_command_buffer` extension. This
PR introduces an extra requirement on the
[CL_COMMAND_BUFFER_SIMULTANEOUS_USE_KHR](https://registry.khronos.org/OpenCL/specs/3.0-unified/html/OpenCL_API.html#CL_COMMAND_BUFFER_SIMULTANEOUS_USE_KHR)
capability being present.

This is based on the [graph execution
wording](https://github.com/intel/llvm/blob/sycl/sycl/doc/extensions/experimental/sycl_ext_oneapi_graph.asciidoc#765-new-handler-member-functions)
on the definition of `handler::ext_oneapi_graph()` that:

> Only one instance of graph will execute at any time. If graph is
submitted multiple times, dependencies are automatically added by the
runtime to prevent concurrent executions of an identical graph.

Such usage results in multiple calls by the SYCL runtime to
`urEnqueueCommandBufferExp` with the same UR command-buffer and event
dependencies to prevent concurrent execution. Without support for
simultaneous-use the OpenCL adapter code cannot guarantee that the first
command-buffer submission has finished execution before it makes
following `clEnqueueCommandBufferKHR` calls with the `cl_event`
decencies. If the first submission is still executing, then an error
will be reported.

Workarounds like adding blocking host waits to the OpenCL UR adapter are
possible, but requiring simultaneous use reflects the vendor
requirements as they are for the currently implementation. I've tried to
document this all in the UR spec and SYCL-Graph design docs, which also
includes a couple of cleanups I found along the way.

Note that the new CTS test fails for Level-Zero adapter, which I've
created intel/llvm#17734 to resolve.

---------

Co-authored-by: Mikołaj Komar <[email protected]>

Author: 2 people

scripts/core/EXP-COMMAND-BUFFER.rst

@@ -58,11 +58,11 @@ to provide additional properties for how the command-buffer should be
 constructed. The members defined in ${x}_exp_command_buffer_desc_t are:
 
 * ``isUpdatable``, which should be set to ``true`` to support :ref:`updating
-command-buffer commands`.
+  command-buffer commands`.
 * ``isInOrder``, which should be set to ``true`` to enable commands enqueued to
-a command-buffer to be executed in an in-order fashion where possible.
+  a command-buffer to be executed in an in-order fashion where possible.
 * ``enableProfiling``, which should be set to ``true`` to enable profiling of
-the command-buffer.
+  the command-buffer.
 
 Command-buffers are reference counted and can be retained and released by
 calling ${x}CommandBufferRetainExp and ${x}CommandBufferReleaseExp respectively.
@@ -226,15 +226,30 @@ Enqueueing Command-Buffers
 Command-buffers are submitted for execution on a ${x}_queue_handle_t with an
 optional list of dependent events. An event is returned which tracks the
 execution of the command-buffer, and will be complete when all appended commands
-have finished executing. It is adapter specific whether command-buffers can be
-enqueued or executed simultaneously, and submissions may be serialized.
+have finished executing.
 
 .. parsed-literal::
     ${x}_event_handle_t executionEvent;
-
     ${x}EnqueueCommandBufferExp(hQueue, hCommandBuffer, 0, nullptr,
                                 &executionEvent);
 
+A command-buffer can be submitted for execution while a previous submission
+of the same command-buffer is still awaiting completion. That is, the user is not
+required to do a blocking wait on the completion of the first command-buffer
+submission before making a second submission of the command-buffer.
+
+Submissions of the same command-buffer should be synchronized to prevent
+concurrent execution. For example, by using events, barriers, or in-order queue
+dependencies. The behavior of multiple submissions of the same command-buffer
+that can execute concurrently is undefined.
+
+.. parsed-literal::
+    // Valid usage if hQueue is in-order but undefined behavior is out-of-order
+    ${x}EnqueueCommandBufferExp(hQueue, hCommandBuffer, 0, nullptr,
+                                nullptr);
+    ${x}EnqueueCommandBufferExp(hQueue, hCommandBuffer, 0, nullptr,
+                                nullptr);
+
 
 Updating Command-Buffer Commands
 --------------------------------------------------------------------------------

source/adapters/opencl/device.cpp

@@ -1531,9 +1531,19 @@ UR_APIEXPORT ur_result_t UR_APICALL urDeviceGetInfo(ur_device_handle_t hDevice,
     CL_RETURN_ON_FAILURE(clGetDeviceInfo(Dev, CL_DEVICE_EXTENSIONS, ExtSize,
                                          ExtStr.data(), nullptr));
 
-    std::string SupportedExtensions(ExtStr.c_str());
-    return ReturnValue(ExtStr.find("cl_khr_command_buffer") !=
-                       std::string::npos);
+    // cl_khr_command_buffer is required for UR command-buffer support
+    cl_device_command_buffer_capabilities_khr Caps = 0;
+    if (ExtStr.find("cl_khr_command_buffer") != std::string::npos) {
+      // A UR command-buffer user needs to be able to enqueue another
+      // submission of the same UR command-buffer without having to manually
+      // check if the first submission has completed.
+      CL_RETURN_ON_FAILURE(
+          clGetDeviceInfo(Dev, CL_DEVICE_COMMAND_BUFFER_CAPABILITIES_KHR,
+                          sizeof(Caps), &Caps, nullptr));
+    }
+
+    return ReturnValue(
+        0 != (Caps & CL_COMMAND_BUFFER_CAPABILITY_SIMULTANEOUS_USE_KHR));
   }
   case UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_CAPABILITIES_EXP: {
     cl_device_id Dev = cl_adapter::cast<cl_device_id>(hDevice);

test/conformance/exp_command_buffer/fill.cpp

@@ -122,6 +122,31 @@ TEST_P(urCommandBufferFillCommandsTest, Buffer) {
   verifyData(output, size);
 }
 
+TEST_P(urCommandBufferFillCommandsTest, ExecuteTwice) {
+  // TODO https://github.com/intel/llvm/issues/17734
+  // Fail on Level-Zero due to blocking wait code in graph_impl.cpp specific
+  // to the level-zero backend that needs moved into the Level-Zero v1 adapter.
+  UUR_KNOWN_FAILURE_ON(uur::LevelZero{});
+  ASSERT_SUCCESS(urCommandBufferAppendMemBufferFillExp(
+      cmd_buf_handle, buffer, pattern.data(), pattern_size, 0, size, 0, nullptr,
+      0, nullptr, &sync_point, nullptr, nullptr));
+
+  std::vector<uint8_t> output(size, 1);
+  ASSERT_SUCCESS(urCommandBufferAppendMemBufferReadExp(
+      cmd_buf_handle, buffer, 0, size, output.data(), 1, &sync_point, 0,
+      nullptr, nullptr, nullptr, nullptr));
+
+  ASSERT_SUCCESS(urCommandBufferFinalizeExp(cmd_buf_handle));
+
+  ASSERT_SUCCESS(
+      urEnqueueCommandBufferExp(queue, cmd_buf_handle, 0, nullptr, nullptr));
+  ASSERT_SUCCESS(
+      urEnqueueCommandBufferExp(queue, cmd_buf_handle, 0, nullptr, nullptr));
+  ASSERT_SUCCESS(urQueueFinish(queue));
+
+  verifyData(output, size);
+}
+
 TEST_P(urCommandBufferFillCommandsTest, USM) {
   ASSERT_SUCCESS(urCommandBufferAppendUSMFillExp(
       cmd_buf_handle, device_ptr, pattern.data(), pattern_size, size, 0,