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

mutable dispatch related fixes #2143

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

Conversation

franz
Copy link
Contributor

@franz franz commented Nov 6, 2024

No description provided.

franz and others added 4 commits November 6, 2024 21:00
…heck

the test checks 'cl_khr_extended_versioning' then immediately calls:
get_extension_version(device, "cl_khr_command_buffer_mutable_dispatch");
.. however, get_extension_version throws an exception when called
with unsupported extension.
If mutable_dispatch extension is supported, the mutable handle
is allowed to be not null. In that case, skip the HandleNotNull
negative tests.
the test uses CL_MUTABLE_DISPATCH_ARGUMENTS_KHR,
but does not check for support.
test MutableDispatchGlobalSize was failing (with an implementation that
doesn't support non-uniform WGs), because the update_global_size = 3
was not a multiple of the local work-size.

Fixed by increasing the global work-size to 256K and update size to 16K.
This should work with all devices that have max_work_group_size <= 16K.

Updates also MutableDispatchWorkGroups which had an out-of-bounds access,
because it hardcoded the value of global work-size.
{
if (CommandFillBaseTest::Skip()) return true;
return is_extension_available(device,
"cl_khr_command_buffer_mutable_dispatch");
Copy link
Contributor

Choose a reason for hiding this comment

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

The mutable-dispatch extension affects kernel commands, not other types of commands. The spec wording for clCommandFillBufferKHR and other non-kernel command appending entry-points is:

  • mutable_handle returns a handle to the command. This parameter is unused, and must be NULL."

The "must be NULL" part isn't conditional on the support for cl_khr_command_buffer_mutable_dispatch, so I don't think skipping here is correct. However, a future cl_khr_command_buffer_mutable_memory_commands extension could relax this for other types of commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants