Skip to content

Make command-buffer creation description parameter not optional #2673

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
EwanC opened this issue Feb 7, 2025 · 1 comment · Fixed by intel/llvm#17058
Closed

Make command-buffer creation description parameter not optional #2673

EwanC opened this issue Feb 7, 2025 · 1 comment · Fixed by intel/llvm#17058
Assignees
@EwanC
Labels
command-buffer Command Buffer feature addition/changes/specification

Comments

@EwanC
Copy link
Contributor

EwanC commented Feb 7, 2025

As discussed in #2670 (comment) the pCommandBufferDesc parameter to urCommandBufferCreateExp is optional. However, we don't specify what the configuration of the created command-buffer is when this isn't passed, and this is also inconsistent with the description parameters to urSamplerCreate & urMemImageCreate which are not optional.

This task is to update the command-buffer spec to make the description non-optional. Note that we always pass the desciptor parameter in the DPC++ callsite in graph_impl.cpp, so there should be no SYCL-RT changes required other than the UR commit bump.
@EwanC EwanC added the command-buffer Command Buffer feature addition/changes/specification label Feb 7, 2025
@EwanC EwanC self-assigned this Feb 7, 2025
@EwanC EwanC mentioned this issue Feb 7, 2025
Closed
EwanC added a commit to Bensuo/unified-runtime that referenced this issue Feb 7, 2025
@EwanC
Make command-buffer creation descriptor mandatory
2128696 
Closes oneapi-src#2673
@EwanC EwanC mentioned this issue Feb 7, 2025
Closed
EwanC added a commit to Bensuo/unified-runtime that referenced this issue Feb 7, 2025
@EwanC
Make command-buffer creation descriptor mandatory
12e0e79 
Closes oneapi-src#2673
EwanC added a commit to Bensuo/unified-runtime that referenced this issue Feb 7, 2025
@EwanC
Make command-buffer creation descriptor mandatory
78d089c 
Closes oneapi-src#2673
@EwanC
Copy link
Contributor Author

EwanC commented Feb 10, 2025

PR with this change here #2676

EwanC added a commit to Bensuo/unified-runtime that referenced this issue Feb 12, 2025
@EwanC
Make command-buffer creation descriptor mandatory
e23d115 
Closes oneapi-src#2673
EwanC added a commit to reble/llvm that referenced this issue Feb 19, 2025
@EwanC
[UR] Make command-buffer creation descriptor mandatory
70d2694 
As discussed in oneapi-src/unified-runtime#2670 (comment) the `pCommandBufferDesc` parameter to `urCommandBufferCreateExp` is optional. However, the UR spec doesn't state what the configuration of the created command-buffer is when this isn't passed, and being optional is also inconsistent with the description parameters to urSamplerCreate & urMemImageCreate which are not optional. This PR updates the descriptor parameter to command-buffer creation to be mandatory to address these concerns.

Closes oneapi-src/unified-runtime#2673
@EwanC EwanC mentioned this issue Feb 19, 2025
Merged
EwanC added a commit to reble/llvm that referenced this issue Feb 19, 2025
@EwanC
[UR] Make command-buffer creation descriptor mandatory
0173bf6 
As discussed in oneapi-src/unified-runtime#2670 (comment) the `pCommandBufferDesc` parameter to `urCommandBufferCreateExp` is optional. However, the UR spec doesn't state what the configuration of the created command-buffer is when this isn't passed, and being optional is also inconsistent with the description parameters to urSamplerCreate & urMemImageCreate which are not optional. This PR updates the descriptor parameter to command-buffer creation to be mandatory to address these concerns.

Closes oneapi-src/unified-runtime#2673
steffenlarsen pushed a commit to intel/llvm that referenced this issue Feb 20, 2025
@EwanC
[UR] Make command-buffer creation descriptor mandatory (#17058)
69941b8 
As discussed in
oneapi-src/unified-runtime#2670 (comment)
the `pCommandBufferDesc` parameter to `urCommandBufferCreateExp` is
optional. However, the UR spec doesn't state what the configuration of
the created command-buffer is when this isn't passed, and being optional
is also inconsistent with the description parameters to urSamplerCreate
& urMemImageCreate which are not optional. This PR updates the
descriptor parameter to command-buffer creation to be mandatory to
address these concerns.

Closes oneapi-src/unified-runtime#2673

**Note**: This UR patch was previously approved and ready-to-merge in
oneapi-src/unified-runtime#2676 prior to the
repo move
kbenzie pushed a commit to kbenzie/unified-runtime that referenced this issue Feb 21, 2025
@EwanC @kbenzie
Make command-buffer creation descriptor mandatory (#17058)
b77fc2b 
As discussed in
oneapi-src#2670 (comment)
the `pCommandBufferDesc` parameter to `urCommandBufferCreateExp` is
optional. However, the UR spec doesn't state what the configuration of
the created command-buffer is when this isn't passed, and being optional
is also inconsistent with the description parameters to urSamplerCreate
& urMemImageCreate which are not optional. This PR updates the
descriptor parameter to command-buffer creation to be mandatory to
address these concerns.

Closes oneapi-src#2673

**Note**: This UR patch was previously approved and ready-to-merge in
oneapi-src#2676 prior to the
repo move
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@EwanC EwanC

Labels
command-buffer Command Buffer feature addition/changes/specification
Projects
None yet
Milestone
No milestone
1 participant
@EwanC