mutable dispatch related fixes by franz · Pull Request #2143 · KhronosGroup/OpenCL-CTS

franz · 2024-11-06T18:54:38Z

No description provided.

…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.

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.

EwanC

Sorry it's taken me a while to re-review this.

My main thought from looking at this again is whether we need to be using as many as 256K and 16K work items for the test, which is quite a big increase on what we had before. I'm not sure if on embedded devices or simulators that would slow the test time down and not really give us that much extra testing coverage of the extension. Could we use work item sizes that are an order of magnitude less but still resolve the issues you've seen?

bashbaug

@franz, are these changes still required? Other than the global work size modifications, I think all of the other fixes have already been made in other PRs (not ideal, obviously, but it is what it is).

bashbaug · 2026-07-07T20:24:55Z

+    const size_t update_global_size = 16 * 1024;
+    const size_t sizeToAllocate = global_work_size * sizeof(cl_int);
+    const size_t num_elements = global_work_size;


The sizeToAllocate issue was already fixed by #2437

Unless update_global_size needs to be modified, I believe these changes are no longer required.

IIRC the update_global_size change was the main motivation of the PR. In the original code, the updated global size (3) is not a divisor of the initial global size (4*16). Is this expected to work even when the implementation doesn't support non-uniform workgroups ?

Note that this test is changing the global work size, not the local work size, and that the local work size recorded into the command buffer is initially nullptr:

OpenCL-CTS/test_conformance/extensions/cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp

Lines 74 to 76 in 3499738

error = clCommandNDRangeKernelKHR(

command_buffer, nullptr, nullptr, kernel, 1, nullptr,

&global_work_size, nullptr, 0, nullptr, nullptr, &command);

It is not modified by the mutable command update, so it remains nullptr:

OpenCL-CTS/test_conformance/extensions/cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_global_size.cpp

Lines 89 to 100 in 3499738

cl_mutable_dispatch_config_khr dispatch_config{

command,

0 /* num_args */,

0 /* num_svm_arg */,

0 /* num_exec_infos */,

0 /* work_dim - 0 means no change to dimensions */,

nullptr /* arg_list */,

nullptr /* arg_svm_list - nullptr means no change*/,

nullptr /* exec_info_list */,

nullptr /* global_work_offset */,

&update_global_size /* global_work_size */,

nullptr /* local_work_size */

Therefore, I don't think there is any requirement to support non-uniform work-groups to run this test. Do you agree?

OK, that part of mutable dispatch was unclear to me. But i agree with your explanation.

Therefore there is no need to keep the update_global_size related changes of this PR, the issue is in our implementation.

If all the other changes are already fixed, we can close this PR.

Cool, if you do find that any of these fixes are still needed, feel free to re-open this PR (or open another new PR if it'd be easier). Thanks!

bashbaug · 2026-07-07T20:32:19Z

+        if ((mutable_capabilities & CL_MUTABLE_DISPATCH_ARGUMENTS_KHR) == 0)
+            return true;


I believe these changes are all covered elsewhere now.

We removed this Skip function in #2498, since it was already covered in a base class.

This check for CL_DEVICE_MUTABLE_DISPATCH_CAPABILITIES has already existed in the Skip check in the base class, see:

OpenCL-CTS/test_conformance/extensions/cl_khr_command_buffer/cl_khr_command_buffer_mutable_dispatch/mutable_command_basic.h

Lines 104 to 108 in 3499738

bool mutable_support =

!clGetDeviceInfo(

device, CL_DEVICE_MUTABLE_DISPATCH_CAPABILITIES_KHR,

sizeof(mutable_capabilities), &mutable_capabilities, nullptr)

&& mutable_capabilities != 0;

franz force-pushed the mutable_dispatch_fixes branch from c26f9eb to 2cdd0eb Compare November 6, 2024 19:00

EwanC requested changes Nov 8, 2024

View reviewed changes

Comment thread test_conformance/extensions/cl_khr_command_buffer/negative_command_buffer_fill.cpp Outdated

franz and others added 3 commits November 27, 2024 16:54

mutable_command_info: fix Skip logic in PropertiesArray test

7484f2d

the test uses CL_MUTABLE_DISPATCH_ARGUMENTS_KHR, but does not check for support.

franz force-pushed the mutable_dispatch_fixes branch from 2cdd0eb to a2fc5d8 Compare November 27, 2024 14:54

franz requested a review from EwanC November 27, 2024 15:53

EwanC reviewed Jan 8, 2025

View reviewed changes

EwanC mentioned this pull request Mar 21, 2025

Enable RISC-V platform support #2344

Open

bashbaug reviewed Jul 7, 2026

View reviewed changes

franz closed this Jul 8, 2026

Navigation Menu

Uh oh!

mutable dispatch related fixes - #2143

mutable dispatch related fixes#2143
franz wants to merge 3 commits into
KhronosGroup:mainfrom
franz:mutable_dispatch_fixes

franz commented Nov 6, 2024

Uh oh!

Uh oh!

EwanC left a comment •

edited

Loading

Uh oh!

bashbaug left a comment

Uh oh!

bashbaug Jul 7, 2026

Uh oh!

franz Jul 8, 2026

Uh oh!

bashbaug Jul 8, 2026

Uh oh!

franz Jul 8, 2026 •

edited

Loading

Uh oh!

bashbaug Jul 8, 2026

Uh oh!

bashbaug Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	error = clCommandNDRangeKernelKHR(
	command_buffer, nullptr, nullptr, kernel, 1, nullptr,
	&global_work_size, nullptr, 0, nullptr, nullptr, &command);

	cl_mutable_dispatch_config_khr dispatch_config{
	command,
	0 /* num_args */,
	0 /* num_svm_arg */,
	0 /* num_exec_infos */,
	0 /* work_dim - 0 means no change to dimensions */,
	nullptr /* arg_list */,
	nullptr /* arg_svm_list - nullptr means no change*/,
	nullptr /* exec_info_list */,
	nullptr /* global_work_offset */,
	&update_global_size /* global_work_size */,
	nullptr /* local_work_size */

		if ((mutable_capabilities & CL_MUTABLE_DISPATCH_ARGUMENTS_KHR) == 0)
		return true;

	bool mutable_support =
	!clGetDeviceInfo(
	device, CL_DEVICE_MUTABLE_DISPATCH_CAPABILITIES_KHR,
	sizeof(mutable_capabilities), &mutable_capabilities, nullptr)
	&& mutable_capabilities != 0;

Uh oh!

Conversation

franz commented Nov 6, 2024

Uh oh!

Uh oh!

EwanC left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bashbaug left a comment

Choose a reason for hiding this comment

Uh oh!

bashbaug Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

franz Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

bashbaug Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

franz Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bashbaug Jul 8, 2026

Choose a reason for hiding this comment

Uh oh!

bashbaug Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

EwanC left a comment •

edited

Loading

franz Jul 8, 2026 •

edited

Loading