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

Add arguments tests for cl_khr_command_buffer_mutable_dispatch. #1749

Merged
merged 23 commits into from
Jul 11, 2023

Conversation

pj87
Copy link
Contributor

@pj87 pj87 commented May 29, 2023

No description provided.

Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
pj87 added 4 commits May 29, 2023 18:06
… arguments test.

Applied review comments for mutable dispatch global arguments test:
- clFinish to ensure command-buffer has finished executing for calling clUpdateMutableCommandsKHR
- Add second clEnqueueCommandBufferKHR for the command-buffer again before calling clEnqueueReadBuffer so that the kernel runs with the new argument

Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
…tch arguments test.

Applied review comments for remaining mutable dispatch arguments test:
- clFinish to ensure command-buffer has finished executing for calling clUpdateMutableCommandsKHR

Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made a bunch of fixes to these tests as a PR to your branch here: pj87#1. Can you check that the fixes were incorporated even if you don't merge the PR? It seems like a lof the issues still remain.

pj87 added 3 commits June 19, 2023 12:16
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Copy link
Contributor

@EwanC EwanC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing the tests pass apart from SVM

/* Set the arguments */
error = clSetKernelArg(kernel, 0, sizeof(cl_mem), &streams[0]);
test_error(error, "Unable to set indexed kernel arguments");
error = clSetKernelArg(kernel, 1, sizeof(cl_mem) * 2, nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why sizeof(cl_mem) * 2 number of bytes of local data rather than based on numberOfInts

pj87 added 6 commits June 30, 2023 13:15
Changes made:
- Remove unneeded headers
- Remove unneeded variable
- Remove unneeded skip condition
- Fix type int -> cl_int

Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
pj87 added 4 commits July 3, 2023 14:58
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
EwanC
EwanC previously approved these changes Jul 4, 2023
Signed-off-by: Paweł Jastrzębski <p.k.jastrzebski@gmail.com>
EwanC
EwanC previously approved these changes Jul 7, 2023
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging as discussed in the July 11th teleconference.

@bashbaug bashbaug merged commit c69bc00 into KhronosGroup:main Jul 11, 2023
6 checks passed
@pj87 pj87 deleted the Mutable-command-arguments branch November 10, 2023 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants