[SYCL][UR][L0 v2] Add a note about make_queue support #19644
Conversation
and return an error if urQueueCreateWithNativeHandle is called with `ze_command_queue_handle_t`. V2 adapter does not support non-immediate or out-of-order command lists. Document this in the extension doc. Currently, there is no way to check whether a command-list is in order or not programatically.
| @@ -388,6 +388,8 @@ If the deprecated variant of <code>backend_input_t<backend::ext_oneapi_level_zer | |||
|
|
|||
| Starting in version 4 of this specification, ```make_queue()``` can be called by passing either a Level Zero ```ze_command_queue_handle_t``` or a Level Zero ```ze_command_list_handle_t```. Queues created from a Level Zero immediate command list (```ze_command_list_handle_t```) generally perform better than queues created from a standard Level Zero ```ze_command_queue_handle_t```. See the Level Zero documentation of these native handles for more details. Also starting in version 4 the ```make_queue()``` function accepts a ```Properties``` member variable. This can contain any of the SYCL properties that are accepted by the SYCL queue constructor, except | |||
| the ```compute_index``` property which is built into the command queue or command list. | |||
|
|
|||
| **Warning:** <span style="color:red"> When L0 v2 adapter is used (when running on platforms with GPUs based on the Xe2 architecture or later, such as Battlemage, Lunar Lake, and Arrow Lake or when SYCL_UR_USE_LEVEL_ZERO_V2=1 is set) ```make_queue()``` accepts only ```ze_command_list_handle_t```: a handle to an **immediate**, **in-order** command list. If non-immediate or out-of-order command list support is needed, the legacy adapter should be used (by setting SYCL_UR_USE_LEVEL_ZERO_V2=0)</span> | |||
There was a problem hiding this comment.
I found this quite hard to read. Something like the below might be clearer:
| **Warning:** <span style="color:red"> When L0 v2 adapter is used (when running on platforms with GPUs based on the Xe2 architecture or later, such as Battlemage, Lunar Lake, and Arrow Lake or when SYCL_UR_USE_LEVEL_ZERO_V2=1 is set) ```make_queue()``` accepts only ```ze_command_list_handle_t```: a handle to an **immediate**, **in-order** command list. If non-immediate or out-of-order command list support is needed, the legacy adapter should be used (by setting SYCL_UR_USE_LEVEL_ZERO_V2=0)</span> | |
| **Warning:** <span style="color:red"> When using the L0 v2 adapter, ```make_queue()``` only accepts ```ze_command_list_handle_t```. The L0 v2 adapter is always used when running on platforms with GPUs based on the Xe2 architecture or later, such as Battlemage, Lunar Lake, and Arrow Lake. Applications may also opt-in to the L0 v2 adapter using the `SYCL_UR_USE_LEVEL_ZERO_V2` environment variable.</span> |
I think you also need to specify what happens if somebody passes a ze_command_queue_handle_t to this function and the V2 adapter is not used. Is it undefined behavior, will the program crash, will we throw an exception...?
More generally... The description of this function's behavior is now really long, with reference to different specification versions and different adapter versions. I think it's too confusing and would benefit from a rewrite, but that might be out of scope for this PR.
@gmlueck - This might be an interesting case to consider in the context of KhronosGroup/SYCL-Docs#777. If the Level Zero backend didn't have to jump through hoops to satisfy the backend_input_t interface, we would be free to define separate overloads like:
sycl::level_zero::make_queue(ze_command_queue_handle_t& handle, ...);
sycl::level_zero::make_queue(ze_command_list_handle_t& handle, ...);I think this would make it much easier to describe what these functions actually do, which would also make it easier for developers to understand and use them.
There was a problem hiding this comment.
Is it undefined behavior, will the program crash, will we throw an exception...?
I think we should throw an exception, so applications know they did something that is not supported.
There was a problem hiding this comment.
Thanks, I applied your suggestion @Pennycook. Right now, the behavior is that when ze_command_queue_handle_t is passed we will throw an exception (UR returns UR_RESULT_ERROR_UNSUPPORTED_FEATURE). However, when the command list is not in-order we might crash. Unfortunately, there is currently no API to check whether a command list is in-order or not. Once this API is added we can also throw an exception in that case.
Should we document this behavior as is right now?
There was a problem hiding this comment.
Yes, I think it would be valuable to capture what the current state is, and to update it later.
If you say that passing an out-of-order command list is undefined behavior, that will cover the current state (crash) and the desired future state (exception). Once we can be sure that we always throw an exception, we can tighten the specification to say that's required.
…ro.md Co-authored-by: John Pennycook <[email protected]>
and return an error if urQueueCreateWithNativeHandle is called with
ze_command_queue_handle_t.V2 adapter does not support non-immediate or out-of-order command lists. Document this in the extension doc.
Currently, there is no way to check whether a command-list is in order or not programatically.