Skip to content

Added test to verify negative result of clSetKernelArg with CL_INVALID_ARG_INDEX #2458

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shajder
Copy link
Contributor

@shajder shajder commented Jul 29, 2025

Related to #2282, according to work plan from here

Copy link
Contributor

@EwanC EwanC left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some nitpicks but LGTM

sampler_arg_kernel = clCreateKernel(program, "sample_test", &error);
test_error(error, "Unable to get sample_test kernel for built program");

// Run the test - CL_INVALID_ARG_INDEX
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't give the reader much extra info as-is, it would add value if instead the comment said that the "sample_test" test kernel has 2 arguments, so 2 is out of range as arguments are indexed from zero.

test_error(error, "Unable to get sample_test kernel for built program");

// Run the test - CL_INVALID_ARG_INDEX
error = clSetKernelArg(sampler_arg_kernel, 2, sizeof(cl_sampler), nullptr);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cl_sampler types aren't used anywhere else in this file, can we use sizeof(cl_mem) to avoid a reader having to work if there's a reason we've started using cl_sampler for this particular test.

{
cl_int error = CL_SUCCESS;
clProgramWrapper program;
clKernelWrapper sampler_arg_kernel;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The kernel names at the top of the file are "sample" rather than "sampler", a "sampler" is a separate concept to what we're trying to test here, so would prefer we didn't complicate the test by using that terminology. Would just call this variable kernel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check that clSetKernelArg called with arg_size of 0 for an argument in the local address space returns CL_INVALID_ARG_SIZE
3 participants