[SYCL][L0] Adds device member to L0 make_queue input type (#6148)

* [SYCL][L0] Adds device member to L0 make_queue input type

This commit adds a new `device` member to the `make_queue` input type for the L0 backend. This allows the L0 backend to correctly associate the right device with the command queue to create the PI queue from, preventing unintended and illegal behavior when the PI queue is later used for otherwise valid operations.

It is currently still valid to create a SYCL queue from a native L0 command queue without associating the correct device, which will lead to the queue being associated with the first device in the supplied SYCL context. This behavior is however marked as deprecated.

Note: The ABI checker test changes were generated with the checker tool, so the ones that are marked as removed are simply just moved by the tool. This is a non-breaking ABI change for the runtime library.

Signed-off-by: Larsen, Steffen <steffen.larsen@intel.com>

Author: steffenlarsen

sycl/doc/extensions/supported/sycl_ext_oneapi_backend_level_zero.md

@@ -14,6 +14,7 @@ This extension provides a feature-test macro as described in the core SYCL speci
 |---|:---|
 |1|Initial extension version.
 |2|Added support for the make_buffer() API.
+|3|Added device member to backend_input_t<backend::ext_oneapi_level_zero, queue>.
 
 NOTE: This extension is following SYCL 2020 backend specification. Prior API for interoperability with Level-Zero is marked
       as deprecated and will be removed in the next release.
@@ -108,8 +109,8 @@ struct {
 ```
 </td>
 </tr><tr>
-<td>queue</td>
-<td><pre>ze_command_queue_handle_t</pre></td>
+<td rowspan="2">queue</td>
+<td rowspan="2"><pre>ze_command_queue_handle_t</pre></td>
 <td>
 
 ``` C++
@@ -119,6 +120,22 @@ struct {
       ext::oneapi::level_zero::ownership::transfer};
 }
 ```
+
+Deprecated as of version 3 of this specification.[^1]
+</td>
+</tr><tr>
+<td>
+
+``` C++
+struct {
+  ze_command_queue_handle_t NativeHandle;
+  device Device;
+  ext::oneapi::level_zero::ownership Ownership{
+      ext::oneapi::level_zero::ownership::transfer};
+}
+```
+
+Supported since version 3 of this specification.[^1]
 </td>
 </tr><tr>
 <td>event</td>
@@ -191,6 +208,8 @@ struct {
 </tr>
 </table>
 
+[^1]: The SYCL implementation is responsible for distinguishing between the variants of <code>backend_input_t<backend::ext_oneapi_level_zero, queue></code>.
+
 ### 4.2 Obtaining of native Level-Zero handles from SYCL objects
                 
 The ```sycl::get_native<backend::ext_oneapi_level_zero>``` free-function is how a raw native Level-Zero handle can be obtained
@@ -275,7 +294,10 @@ make_queue<backend::ext_oneapi_level_zero>(
     const context &Context)
 ```
 </td>
-<td>Constructs a SYCL queue instance from a Level-Zero <code>ze_command_queue_handle_t</code>. The <code>Context</code> argument must be a valid SYCL context encapsulating a Level-Zero context. The queue is attached to the first device in the passed SYCL context. The <code>Ownership</code> input structure member specifies if the SYCL runtime should take ownership of the passed native handle. The default behavior is to transfer the ownership to the SYCL runtime. See section 4.4 for details.</td>
+<td>Constructs a SYCL queue instance from a Level-Zero <code>ze_command_queue_handle_t</code>. The <code>Context</code> argument must be a valid SYCL context encapsulating a Level-Zero context. The <code>Device</code> input structure member specifies the device to create the <code>queue</code> against and must be in <code>Context</code>. The <code>Ownership</code> input structure member specifies if the SYCL runtime should take ownership of the passed native handle. The default behavior is to transfer the ownership to the SYCL runtime. See section 4.4 for details.
+
+If the deprecated variant of <code>backend_input_t<backend::ext_oneapi_level_zero, queue></code> is passed to <code>make_queue</code> the queue is attached to the first device in <code>Context</code>.
+</td>
 </tr><tr>
 <td>
 
@@ -485,3 +507,4 @@ struct free_memory {
 |6|2021-08-30|Dmitry Vodopyanov|Updated according to SYCL 2020 reqs for extensions
 |7|2021-09-13|Sergey Maslov|Updated according to SYCL 2020 standard
 |8|2022-01-06|Artur Gainullin|Introduced make_buffer() API
+|9|2022-05-12|Steffen Larsen|Added device member to queue input type

sycl/include/CL/sycl/backend.hpp

@@ -208,6 +208,11 @@ __SYCL_EXPORT device make_device(pi_native_handle NativeHandle,
 __SYCL_EXPORT context make_context(pi_native_handle NativeHandle,
                                    const async_handler &Handler,
                                    backend Backend);
+__SYCL_EXPORT queue make_queue(pi_native_handle NativeHandle,
+                               const context &TargetContext,
+                               const device &TargetDevice, bool KeepOwnership,
+                               const async_handler &Handler, backend Backend);
+// TODO: Unused. Remove when allowed.
 __SYCL_EXPORT queue make_queue(pi_native_handle NativeHandle,
                                const context &TargetContext, bool KeepOwnership,
                                const async_handler &Handler, backend Backend);

sycl/include/CL/sycl/detail/backend_traits_level_zero.hpp

@@ -24,6 +24,7 @@
 #include <CL/sycl/kernel_bundle.hpp>
 #include <CL/sycl/queue.hpp>
 #include <sycl/ext/oneapi/backend/level_zero_ownership.hpp>
+#include <sycl/ext/oneapi/filter_selector.hpp>
 
 typedef struct _ze_command_queue_handle_t *ze_command_queue_handle_t;
 typedef struct _ze_context_handle_t *ze_context_handle_t;
@@ -38,6 +39,9 @@ __SYCL_INLINE_NAMESPACE(cl) {
 namespace sycl {
 namespace detail {
 
+// Forward declarations
+class device_impl;
+
 // TODO the interops for context, device, event, platform and program
 // may be removed after removing the deprecated 'get_native()' methods
 // from the corresponding classes. The interop<backend, queue> specialization
@@ -130,11 +134,61 @@ template <> struct BackendReturn<backend::ext_oneapi_level_zero, event> {
   using type = ze_event_handle_t;
 };
 
+struct OptionalDevice {
+  OptionalDevice() : DeviceImpl(nullptr) {}
+  OptionalDevice(device dev) : DeviceImpl(getSyclObjImpl(dev)) {}
+
+  operator device() const {
+    if (!DeviceImpl)
+      throw runtime_error("No device has been set.", PI_INVALID_DEVICE);
+    return createSyclObjFromImpl<device>(DeviceImpl);
+  }
+
+  OptionalDevice &operator=(OptionalDevice &Other) {
+    DeviceImpl = Other.DeviceImpl;
+    return *this;
+  }
+  OptionalDevice &operator=(device &Other) {
+    DeviceImpl = getSyclObjImpl(Other);
+    return *this;
+  }
+
+private:
+  std::shared_ptr<device_impl> DeviceImpl;
+
+  friend bool OptionalDeviceHasDevice(const OptionalDevice &Dev);
+};
+
+// Inspector function in the detail namespace to avoid exposing
+// OptionalDevice::hasDevice to user-space.
+inline bool OptionalDeviceHasDevice(const OptionalDevice &Dev) {
+  return Dev.DeviceImpl != nullptr;
+}
+
 template <> struct BackendInput<backend::ext_oneapi_level_zero, queue> {
   struct type {
     interop<backend::ext_oneapi_level_zero, queue>::type NativeHandle;
-    ext::oneapi::level_zero::ownership Ownership{
-        ext::oneapi::level_zero::ownership::transfer};
+    ext::oneapi::level_zero::ownership Ownership;
+
+    // TODO: Change this to be device when the deprecated constructor is
+    // removed.
+    OptionalDevice Device;
+
+    type()
+        : Ownership(ext::oneapi::level_zero::ownership::transfer), Device() {}
+
+    __SYCL_DEPRECATED("Use backend_input_t<backend::ext_oneapi_level_zero, "
+                      "queue> constructor with device parameter")
+    type(interop<backend::ext_oneapi_level_zero, queue>::type nativeHandle,
+         ext::oneapi::level_zero::ownership ownership =
+             ext::oneapi::level_zero::ownership::transfer)
+        : NativeHandle(nativeHandle), Ownership(ownership), Device() {}
+
+    type(interop<backend::ext_oneapi_level_zero, queue>::type nativeHandle,
+         device dev,
+         ext::oneapi::level_zero::ownership ownership =
+             ext::oneapi::level_zero::ownership::transfer)
+        : NativeHandle(nativeHandle), Ownership(ownership), Device(dev) {}
   };
 };
 

sycl/include/CL/sycl/detail/pi.h

@@ -44,10 +44,11 @@
 // piQueueFlush function.
 // 7.9 Added new context and ownership arguments to
 // piextMemCreateWithNativeHandle.
+// 8.10 Added new optional device argument to piextQueueCreateWithNativeHandle
 //
 #include "CL/cl.h"
-#define _PI_H_VERSION_MAJOR 7
-#define _PI_H_VERSION_MINOR 9
+#define _PI_H_VERSION_MAJOR 8
+#define _PI_H_VERSION_MINOR 10
 
 #define _PI_STRING_HELPER(a) #a
 #define _PI_CONCAT(a, b) _PI_STRING_HELPER(a.b)
@@ -1158,12 +1159,15 @@ piextQueueGetNativeHandle(pi_queue queue, pi_native_handle *nativeHandle);
 ///
 /// \param nativeHandle is the native handle to create PI queue from.
 /// \param context is the PI context of the queue.
-/// \param queue is the PI queue created from the native handle.
+/// \param device is the PI device associated with the native device used when
+///   creating the native queue. This parameter is optional but some backends
+///   may fail to create the right PI queue if omitted.
 /// \param pluginOwnsNativeHandle Indicates whether the created PI object
 ///        should take ownership of the native handle.
+/// \param queue is the PI queue created from the native handle.
 __SYCL_EXPORT pi_result piextQueueCreateWithNativeHandle(
-    pi_native_handle nativeHandle, pi_context context, pi_queue *queue,
-    bool pluginOwnsNativeHandle);
+    pi_native_handle nativeHandle, pi_context context, pi_device device,
+    bool pluginOwnsNativeHandle, pi_queue *queue);
 
 //
 // Memory
@@ -1823,9 +1827,9 @@ struct _pi_plugin {
   // Some choices are:
   // - Use of integers to keep major and minor version.
   // - Keeping char* Versions.
-  char PiVersion[4];
+  char PiVersion[10];
   // Plugin edits this.
-  char PluginVersion[4];
+  char PluginVersion[10];
   char *Targets;
   struct FunctionPointers {
 #define _PI_API(api) decltype(::api) *api;

sycl/include/CL/sycl/feature_test.hpp.in

@@ -69,7 +69,7 @@ namespace sycl {
 #define SYCL_EXT_INTEL_MEM_CHANNEL_PROPERTY 1
 #define SYCL_EXT_INTEL_USM_ADDRESS_SPACES 1
 #define SYCL_EXT_INTEL_RUNTIME_BUFFER_LOCATION 1
-#define SYCL_EXT_ONEAPI_BACKEND_LEVEL_ZERO 2
+#define SYCL_EXT_ONEAPI_BACKEND_LEVEL_ZERO 3
 #define SYCL_EXT_ONEAPI_USM_DEVICE_READ_ONLY 1
 #cmakedefine01 SYCL_BUILD_PI_CUDA
 #if SYCL_BUILD_PI_CUDA

sycl/include/sycl/ext/oneapi/backend/level_zero.hpp

@@ -31,9 +31,13 @@ __SYCL_EXPORT context make_context(const std::vector<device> &DeviceList,
 __SYCL_EXPORT program make_program(const context &Context,
                                    pi_native_handle NativeHandle);
 #endif
+__SYCL_DEPRECATED("Use make_queue with device parameter")
 __SYCL_EXPORT queue make_queue(const context &Context,
                                pi_native_handle InteropHandle,
                                bool keep_ownership = false);
+__SYCL_EXPORT queue make_queue(const context &Context, const device &Device,
+                               pi_native_handle InteropHandle,
+                               bool keep_ownership = false);
 __SYCL_EXPORT event make_event(const context &Context,
                                pi_native_handle InteropHandle,
                                bool keep_ownership = false);
@@ -136,8 +140,11 @@ inline queue make_queue<backend::ext_oneapi_level_zero>(
     const backend_input_t<backend::ext_oneapi_level_zero, queue> &BackendObject,
     const context &TargetContext, const async_handler Handler) {
   (void)Handler;
+  const device Device = detail::OptionalDeviceHasDevice(BackendObject.Device)
+                            ? device{BackendObject.Device}
+                            : TargetContext.get_devices()[0];
   return ext::oneapi::level_zero::make_queue(
-      TargetContext,
+      TargetContext, Device,
       detail::pi::cast<pi_native_handle>(BackendObject.NativeHandle),
       BackendObject.Ownership == ext::oneapi::level_zero::ownership::keep);
 }

sycl/plugins/cuda/pi_cuda.cpp

@@ -2443,8 +2443,8 @@ pi_result cuda_piextQueueGetNativeHandle(pi_queue queue,
 ///
 /// \return TBD
 pi_result cuda_piextQueueCreateWithNativeHandle(pi_native_handle, pi_context,
-                                                pi_queue *,
-                                                bool ownNativeHandle) {
+                                                pi_device, bool ownNativeHandle,
+                                                pi_queue *) {
   (void)ownNativeHandle;
   cl::sycl::detail::pi::die(
       "Creation of PI queue from native handle not implemented");
@@ -5053,7 +5053,10 @@ pi_result piPluginInit(pi_plugin *PluginInit) {
   }
 
   // PI interface supports higher version or the same version.
-  strncpy(PluginInit->PluginVersion, SupportedVersion, 4);
+  size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
+  if (strlen(SupportedVersion) >= PluginVersionSize)
+    return PI_INVALID_VALUE;
+  strncpy(PluginInit->PluginVersion, SupportedVersion, PluginVersionSize);
 
   // Set whole function table to zero to make it easier to detect if
   // functions are not set up below.

sycl/plugins/esimd_emulator/pi_esimd_emulator.cpp

@@ -1000,7 +1000,7 @@ pi_result piextQueueGetNativeHandle(pi_queue, pi_native_handle *) {
 }
 
 pi_result piextQueueCreateWithNativeHandle(pi_native_handle, pi_context,
-                                           pi_queue *, bool) {
+                                           pi_device, bool, pi_queue *) {
   DIE_NO_IMPLEMENTATION;
 }
 

sycl/plugins/hip/pi_hip.cpp

@@ -2310,10 +2310,12 @@ pi_result hip_piextQueueGetNativeHandle(pi_queue queue,
 /// \return TBD
 pi_result hip_piextQueueCreateWithNativeHandle(pi_native_handle nativeHandle,
                                                pi_context context,
-                                               pi_queue *queue,
-                                               bool ownNativeHandle) {
+                                               pi_device device,
+                                               bool ownNativeHandle,
+                                               pi_queue *queue) {
   (void)nativeHandle;
   (void)context;
+  (void)device;
   (void)queue;
   (void)ownNativeHandle;
   cl::sycl::detail::pi::die(
@@ -4892,7 +4894,10 @@ pi_result piPluginInit(pi_plugin *PluginInit) {
   }
 
   // PI interface supports higher version or the same version.
-  strncpy(PluginInit->PluginVersion, SupportedVersion, 4);
+  size_t PluginVersionSize = sizeof(PluginInit->PluginVersion);
+  if (strlen(SupportedVersion) >= PluginVersionSize)
+    return PI_INVALID_VALUE;
+  strncpy(PluginInit->PluginVersion, SupportedVersion, PluginVersionSize);
 
   // Set whole function table to zero to make it easier to detect if
   // functions are not set up below.

sycl/plugins/level_zero/pi_level_zero.cpp

@@ -3468,8 +3468,9 @@ pi_result piextQueueGetNativeHandle(pi_queue Queue,
 }
 
 pi_result piextQueueCreateWithNativeHandle(pi_native_handle NativeHandle,
-                                           pi_context Context, pi_queue *Queue,
-                                           bool OwnNativeHandle) {
+                                           pi_context Context, pi_device Device,
+                                           bool OwnNativeHandle,
+                                           pi_queue *Queue) {
   PI_ASSERT(Context, PI_INVALID_CONTEXT);
   PI_ASSERT(NativeHandle, PI_INVALID_VALUE);
   PI_ASSERT(Queue, PI_INVALID_QUEUE);
@@ -3478,9 +3479,11 @@ pi_result piextQueueCreateWithNativeHandle(pi_native_handle NativeHandle,
   // Assume this is the "0" index queue in the compute command-group.
   std::vector<ze_command_queue_handle_t> ZeQueues{ZeQueue};
 
-  // Attach the queue to the "0" device.
-  // TODO: see if we need to let user choose the device.
-  pi_device Device = Context->Devices[0];
+  // For compatibility with older implementations we allow the device to be
+  // optional for now. Once the deprecated interop API is removed this can be
+  // changed to an assert(Device).
+  if (!Device)
+    Device = Context->Devices[0];
   // TODO: see what we can do to correctly initialize PI queue for
   // compute vs. copy Level-Zero queue. Currently we will send
   // all commands to the "ZeQueue".