Improve specification of command-buffer update errors #2462
Conversation
github-actions
bot
added
loader
EwanC
force-pushed
the
cmd-buf_update_errors
branch
from
664febd to
c166e74
Compare
EwanC
changed the title
EwanC
force-pushed
the
cmd-buf_update_errors
branch
from
c166e74 to
00ca890
Compare
Bump UR tag to [Improve specification of command-buffer update errors](oneapi-src/unified-runtime#2462)
| @@ -1203,11 +1204,15 @@ params: | |||
| desc: "[in] Struct defining how the kernel command is to be updated." | |||
| returns: | |||
| - $X_RESULT_ERROR_UNSUPPORTED_FEATURE: | |||
| - "If update functionality is not supported by the device." | |||
| - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_KERNEL_ARGUMENTS is not supported by the device, but any of `pUpdateKernelLaunch->numNewMemObjArgs`, `pUpdateKernelLaunch->numNewPointerArgs`, or `pUpdateKernelLaunch->numNewValueArgs` are not zero." | |||
| - "If $X_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE is not supported by the device but `pUpdateKernelLaunch->pNewLocalWorkSize` is not nullptr." | |||
There was a problem hiding this comment.
In the update descriptor wording we have the following:
If
pNewGlobalWorkSizeis set andpNewLocalWorkSizeis nullptr,
then the runtime implementation will choose the local work size.
If the device doesn't support UR_DEVICE_COMMAND_BUFFER_UPDATE_CAPABILITY_FLAG_LOCAL_WORK_SIZE are we just assuming that the implementation won't generate a new local work size here (which may be different from the previous value) or that it would do that but throw an error because updating the local size is not possible?
If the former we should probably change that quoted wording to reflect that it is not guaranteed to generate a new local size if not supported, or document the latter error here.
There was a problem hiding this comment.
Good shout, I went with an error because the old local size might not be compatable with the new global size if we can't update it under the hood.
The `UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXP` query no longer exists, so error defined for trying to make an updatable command-buffer when a device doesn't support it needs reworded to say if no capabilities are supported. Additionally, for each individual update capability, specify that an error is thrown if update is attempted using that characteristic. UR CTS added to verify this.
Return unsupported when a device doesn't support local memory update but attempts the following usage > If pNewGlobalWorkSize is set and pNewLocalWorkSize is nullptr, then the runtime implementation will choose the local work size.
EwanC
force-pushed
the
cmd-buf_update_errors
branch
from
00ca890 to
7d4d09c
Compare
There was a problem hiding this comment.
Putting back into draft because I think we need more granular checking in this adapter for the unsupported cases in update()
The
UR_DEVICE_INFO_COMMAND_BUFFER_UPDATE_SUPPORT_EXPquery no longer exists, so error defined for trying to make an updatable command-buffer when a device doesn't support it needs reworded to say if no capabilities are supported.Additionally, for each individual update capability, specify that an error is thrown if update is attempted using that characteristic.
UR CTS added to verify the above clarifications.
DPC++ PR intel/llvm#16383