Add clSetCommandQueueProperty test

[ellnor01]

Add test to check that clSetCommandQueueProperty
successfully changes command queue properties.

Fixes #904

Signed-off-by: Ellen Norris-Thompson ellen.norris-thompson@arm.com

[gwawiork]

Hi
I am not sure why we want to test this API call . According to specification it is deprecated:
https://www.khronos.org/registry/OpenCL/specs/3.0-unified/html/OpenCL_API.html#clSetCommandQueueProperty

[kpet]

Deprecated doesn't mean removed. All OpenCL implementations still have to support it. We've discussed removing it in the working group but until that happens, we have to test it.

[gwawiork]

For now I got CL_INVALID_VALUE because properties are broken

[bashbaug]

FWIW, we've had this comment in the OpenCL headers for as long as I can remember:

#ifdef CL_USE_DEPRECATED_OPENCL_1_0_APIS
    /*
     *  WARNING:
     *     This API introduces mutable state into the OpenCL implementation. It has been REMOVED
     *  to better facilitate thread safety.  The 1.0 API is not thread safe. It is not tested by the
     *  OpenCL 1.1 conformance test, and consequently may not work or may not work dependably.
     *  It is likely to be non-performant. Use of this API is not advised. Use at your own risk.
     *
     *  Software developers previously relying on this API are instructed to set the command queue
     *  properties when creating the queue, instead.
     */
    extern CL_API_ENTRY cl_int CL_API_CALL
    clSetCommandQueueProperty(cl_command_queue              command_queue,
                              cl_command_queue_properties   properties,
                              cl_bool                       enable,
                              cl_command_queue_properties * old_properties) CL_API_SUFFIX__VERSION_1_0_DEPRECATED;
#endif /* CL_USE_DEPRECATED_OPENCL_1_0_APIS */

I think it's fine to have a CTS test that ensures the function can be called and things don't blow up, but I don't expect that it will work and change the command-queue properties on all currently shipping implementations. Our GPU implementation unconditionally returns CL_INVALID_VALUE for this API, for example.

I'm all for clarifying the spec to say this more clearly, as I agree this is a bit stronger than "deprecated".

[alycm]

We also never implemented this entry point and I'm not aware of that ever causing problems. I understand that is not consistent with the specification definition of deprecation though.

[gwawiork]

For now I got CL_INVALID_VALUE because properties are broken

I've removed my previous comments. I've analyzed the test once again and my comments were wrong. The test returns always CL_INVALID_VALUE on our GPU and properties are correct

[ellnor01]

Waiting for feedback on Docs-621 before making significant changes to this test.
Parking this unless there is a specific fix the test which can be resolved in the meantime?

[ahesham-arm]

@kpet Rebased and CI checks are passing.

[bashbaug]

I think we need to update this test to allow OpenCL implementations newer than OpenCL 1.0 to unconditionally return an error code for this function, see KhronosGroup/OpenCL-Docs#980.

[kpet]

@ahesham-arm Agree with Ben. This test triggered a few discussions and we ended up allowing non-1.0 implementations to just not support this function. There are two options I can think of:

1. Skip this test on non-1.0 implementations (we might also want to check that it does not use post-1.0 features)
2. Skip the test at the first call to clSetCommandQueueProperty that returns an error.

I think my preference is for (2) since that seamlessly allows the test to run to completion on implementations that do want to support this function and skip it on others. Implementers can decide to expect pass or skip as a good result.

[ahesham-arm]

@bashbaug @kpet Thanks for the review. Added the following after the first call to clSetCommandQueueProperty:

if (err == CL_INVALID_OPERATION)
{
    // Implementations are allowed to return an error for
    // non-OpenCL 1.0 devices. In which case, skip the test.
    return TEST_SKIPPED_ITSELF;
}

[bashbaug]

Merging as discussed in the August 6th teleconference.