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

command buffer negative_create_command_buffer_not_supported_properties is incorrect #2247

Open
bashbaug opened this issue Jan 26, 2025 · 0 comments · May be fixed by #2248
Open

command buffer negative_create_command_buffer_not_supported_properties is incorrect #2247

bashbaug opened this issue Jan 26, 2025 · 0 comments · May be fixed by #2248

Comments

@bashbaug
Copy link
Contributor

I don't believe that the command buffer negative_create_command_buffer_not_supported_properties test is correct.

This test is trying to test this specific error code:

CL_INVALID_PROPERTY if values specified in properties are valid but are not supported by all the devices associated with command-queues in queues.

It does so by trying to find a "valid but unsupported property", specifically:

        bool skip = true;
        if (!simultaneous_use_support)
        {
            unsupported_prop = CL_COMMAND_BUFFER_SIMULTANEOUS_USE_KHR;
            skip = false;
        }
        else if (!device_side_enqueue_support)
        {
            unsupported_prop = CL_COMMAND_BUFFER_DEVICE_SIDE_SYNC_KHR;
            skip = false;
        }

Note that device_side_enqueue_support was queried using:

    cl_device_command_buffer_capabilities_khr capabilities;
    error = clGetDeviceInfo(device, CL_DEVICE_COMMAND_BUFFER_CAPABILITIES_KHR,
                            sizeof(capabilities), &capabilities, NULL);
    // snip
    device_side_enqueue_support =
        (capabilities & CL_COMMAND_BUFFER_CAPABILITY_DEVICE_SIDE_ENQUEUE_KHR)
        != 0;

There are two problems here:

  1. For implementations that do not support cl_khr_command_buffer_multi_device, the CL_COMMAND_BUFFER_DEVICE_SIDE_SYNC_KHR enum does not exist. This means that this property is "not valid" rather than "not supported", therefore the error code should be CL_INVALID_VALUE rather than CL_INVALID_PROPERTY.
  2. The device_side_enqueue_support capability is unrelated to CL_COMMAND_BUFFER_DEVICE_SIDE_SYNC_KHR.

Recommended fixes are:

  1. Remove the codepath that tests the "unsupported property" CL_COMMAND_BUFFER_DEVICE_SIDE_SYNC_KHR. This means that the test will skip when simultaneous use is supported, because the only valid property is supported.
  2. Revisit all of the cases that check device_side_enqueue_support to be sure this was the intended check.
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 a pull request may close this issue.

1 participant