rethink test_cl_khr_command_buffer command_buffer_wait_for_sec_command_buffer

[bashbaug]

We should rethink how to test:

rethink test_cl_khr_command_buffer command_buffer_wait_for_sec_command_buffer

The current test has no data dependency between the two command-buffers, so I don't believe the current test is testing what it is intending to test.

If we want to have a command-buffer in one command-queue depend on a command-buffer in a different queue then we probably need a data dependency between the commands in each command-buffer. For example, in the first command-buffer we can copy from buffer A to buffer B, and then the second command buffer can copy from buffer B to buffer C, and we can check the results of buffer C. Or, perhaps the first command-buffer can modify buffer A in place (say, by adding a value to each element of buffer A), and then the second command-buffer can also modify buffer A in place (say, by adding a different value to each element of buffer A), and we can check that both updates have been applied. There are lots of options.

Here was the original PR comment that prompted this issue:

Originally posted by @bashbaug in https://github.com/KhronosGroup/OpenCL-CTS/pull/1840#discussion_r1781917261

I don't think this is necessarily wrong, but I'm having a tough time figuring out what this test is doing now.

As far as I can tell, for setup:

1. There are two queues (queue and queue_sec), two command buffers (command_buffer and command_buffer_sec), two kernels (kernel and kernel_sec), and two sets of buffers used for kernel inputs and outputs (in_mem, out_mem, off_mem, in_mem_sec, out_mem_sec, and off_mem_sec).
   • Note: queue_sec is exclusively in-order. queue may be out-of-order.
2. in_mem, out_mem, and off_mem are set as kernel arguments for kernel and recorded into command_buffer.
3. in_mem_sec, out_mem_sec, and off_mem_sec are set as kernel arguments for kernel_sec and recorded into command_buffer_sec.
4. command_buffer records kernel, and command_buffer_sec records kernel_sec.

Then, when the test executes:

5. In queue_sec, we initialize in_mem_sec, then enqueue command_buffer_sec. We save the event for command_buffer_sec.
   • Remember: command_buffer_sec recorded kernel_sec, which reads in_mem_sec and writes out_mem_sec.
6. In queue, we initialize in_mem, then enqueue command_buffer, with an event dependency on the initialization and the execution of command_buffer_sec in queue_sec. We save the event for command_buffer.
   • Remember: command_buffer recorded kernel, which reads in_mem and writes out_mem.

To verify the test output:

7. We enqueue a read from out_mem with an event dependency on the execution of command_buffer.
8. We flush queue, then finish queue_sec, then finish queue.
9. We check the results that we read.

This is all nominally fine, but we never check the results of out_mem_sec or anything that executes in command_buffer_sec, really. Is that what is intended?

Another possible solution that wouldn't require a second kernel and a second set of memory objects is to have the initialization of in_mem in step (6) depend on the execution of the command buffer in step (5). This ensures that in_mem will not be overwritten while the command buffer is executing. I'm not sure if that's any better, though...