-
Notifications
You must be signed in to change notification settings - Fork 198
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 global size tests for cl_khr_command_buffer_mutable_dispatch. #1744
Add global size tests for cl_khr_command_buffer_mutable_dispatch. #1744
Conversation
Signed-off-by: Paweł Jastrzębski <[email protected]>
...cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp
Outdated
Show resolved
Hide resolved
We should check that's some observable output from the kernel as a result of the change to global work size, not just that clGetMutableCommandInfoKHR has been updated. For example, getting every work-item to call get_global_size() inside of the kernel and writing it to a buffer, then reading the buffer after the command-buffer enqueue has finished and check it matches what we expect. Signed-off-by: Paweł Jastrzębski <[email protected]>
...cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp
Show resolved
Hide resolved
...cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp
Outdated
Show resolved
Hide resolved
Applied review comments for mutable dispatch global arguments test: - clFinish to ensure command-buffer has finished executing for calling clUpdateMutableCommandsKHR - Change variable and constant names for global size Signed-off-by: Paweł Jastrzębski <[email protected]>
Changes made: - Fix skip conditions - Remove obsolete variable - Replace a variable with a constant Signed-off-by: Paweł Jastrzębski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp
Outdated
Show resolved
Hide resolved
size_t info_global_size = 0; | ||
const size_t update_global_size = 3; | ||
const size_t sizeToAllocate = 64; | ||
const size_t num_elements = sizeToAllocate / sizeof(cl_int); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick: I think these variables are only used inside of Run
- do they need to be class member variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to put it there for more tidy code, since they are mostly const
s.
Changes made: - Remove explicit base class call - Fix condition check - Fix constant magic number Signed-off-by: Paweł Jastrzębski <[email protected]>
Signed-off-by: Paweł Jastrzębski <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs rebased to remove conflicts, but happy with test content.
...cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp
Outdated
Show resolved
Hide resolved
Signed-off-by: Paweł Jastrzębski <[email protected]>
There was a problem hiding this 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.
No description provided.