Add device-side execution test for cl_khr_command_buffer#2657
Open
shajder wants to merge 2 commits intoKhronosGroup:mainfrom
Open
Add device-side execution test for cl_khr_command_buffer#2657shajder wants to merge 2 commits intoKhronosGroup:mainfrom
shajder wants to merge 2 commits intoKhronosGroup:mainfrom
Conversation
Adds a test verifying that a command-buffer can record and replay a kernel which performs device-side enqueue via enqueue_kernel()
EwanC
reviewed
Apr 13, 2026
EwanC
approved these changes
Apr 13, 2026
Contributor
EwanC
left a comment
EwanC
left a comment
There was a problem hiding this comment.
I haven't got a device handy that can run this test but LGTM
bashbaug
requested changes
Apr 28, 2026
Contributor
bashbaug
left a comment
bashbaug
left a comment
There was a problem hiding this comment.
I didn't do a very thorough review but I did ensure that this test works on the Intel CPU OpenCL device when the OpenCL C version issue is fixed.
Comment on lines
+121
to
+129
| error = create_single_kernel_helper_create_program(context, &program, 1, | ||
| &kernel_str); | ||
| test_error(error, "Failed to create program with source"); | ||
|
|
||
| error = clBuildProgram(program, 1, &device, nullptr, nullptr, nullptr); | ||
| test_error(error, "Failed to build program"); | ||
|
|
||
| kernel = clCreateKernel(program, "device_enqueue", &error); | ||
| test_error(error, "Failed to create device execution kernel"); |
Contributor
There was a problem hiding this comment.
This is overly verbose and it doesn't work - device-side enqueue requires OpenCL C 2.0 or newer, and passing nullptr as build options means we're getting the default latest version of OpenCL C 1.x.
How about instead:
Suggested change
| error = create_single_kernel_helper_create_program(context, &program, 1, | |
| &kernel_str); | |
| test_error(error, "Failed to create program with source"); | |
| error = clBuildProgram(program, 1, &device, nullptr, nullptr, nullptr); | |
| test_error(error, "Failed to build program"); | |
| kernel = clCreateKernel(program, "device_enqueue", &error); | |
| test_error(error, "Failed to create device execution kernel"); | |
| error = create_single_kernel_helper(context, &program, &kernel, 1, | |
| &kernel_str, "device_enqueue"); | |
| test_error(error, "Failed to create test kernel"); |
|
|
||
| } // anonymous namespace | ||
|
|
||
| REGISTER_TEST(device_execution) |
Contributor
There was a problem hiding this comment.
Nitpick: I think the name of this test is misleading, since almost all of the command buffer tests (except maybe the negative tests) are executing on the device.
Should the name of this test be "device_enqueue" instead?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related to #1667