Skip to content
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.

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

[CommandBuffer] Make double finalizing fail #2327

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

hdelan
Copy link
Contributor

@hdelan hdelan commented Nov 14, 2024

A command buffer should fail if finalize is called multiple times on the same command buffer. This matches openCL behaviour.

DPC++ PR: intel/llvm#16097

@hdelan hdelan requested a review from a team as a code owner November 14, 2024 16:06
@github-actions github-actions bot added conformance Conformance test suite issues. level-zero L0 adapter specific issues cuda CUDA adapter specific issues hip HIP adapter specific issues command-buffer Command Buffer feature addition/changes/specification labels Nov 14, 2024
@github-actions github-actions bot added specification Changes or additions to the specification experimental Experimental feature additions/changes/specification labels Nov 14, 2024
@hdelan hdelan requested a review from a team as a code owner November 14, 2024 17:06
@github-actions github-actions bot added the loader Loader related feature/bug label Nov 14, 2024
A command buffer should fail if finalize is called multiple times on the
same command buffer. This matches openCL behaviour.
urCommandBufferFinalizeExp will return UR_RESULT_ERROR_INVALID_OPERATION
if the command buffer has already been finalized.
Don't need extra flag for IsInitialized if checking if cudaGraphExec is
no longer its initial value of nullptr.
@hdelan hdelan added the ready to merge Added to PR's which are ready to merge label Nov 15, 2024
@EwanC
Copy link
Contributor

EwanC commented Nov 15, 2024

Can you whack a link to the DPC++ PR for this in the description, ta

@hdelan hdelan removed the ready to merge Added to PR's which are ready to merge label Nov 15, 2024
@hdelan hdelan added the ready to merge Added to PR's which are ready to merge label Nov 19, 2024
@callumfare
Copy link
Contributor

@hdelan This PR is next in the queue to be merged, could you resolve the conflict?

@EwanC
Copy link
Contributor

EwanC commented Nov 22, 2024

@hdelan This PR is next in the queue to be merged, could you resolve the conflict?

Hugh is on holiday until Monday, so might be better to jump to another ready-to-merge PR until then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
command-buffer Command Buffer feature addition/changes/specification conformance Conformance test suite issues. cuda CUDA adapter specific issues experimental Experimental feature additions/changes/specification hip HIP adapter specific issues level-zero L0 adapter specific issues loader Loader related feature/bug ready to merge Added to PR's which are ready to merge specification Changes or additions to the specification
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants