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

Added test for work-item functions with out-of-range arguments #2099

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

shajder
Copy link
Contributor

@shajder shajder commented Oct 1, 2024

Fixes #2005 according to task description.

svenvh
svenvh previously approved these changes Nov 5, 2024

const char *outOfRangeWorkItemKernelCodeExt =
R"(
uint dimindx=get_work_dim()+1;
Copy link
Contributor

Choose a reason for hiding this comment

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

A smart compiler would still be able to prove at compile time that this dimidx is out-of-range.

If you want to be sure the compiler cannot prove that the dimidx is out-of-range, it'd be better to pass it as a kernel argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Argument passed


int test_work_item_functions(cl_device_id deviceID, cl_context context, cl_command_queue queue, int num_elements)
const char *outOfRangeWorkItemKernelCodeExt11 =
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use a single kernel for this stuff, and guard the parts that are specific to newer OpenCL C versions using __OPENCL_VERSION__ or some other preprocessor mechanism? I think that would be easier to understand and maintain vs. stitching together these sample kernels with sprintf.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, variants replaced with __OPENCL_VERSION__ preprocessor conditions

kernel_source(src), max_workgroup_size(0)
{}

virtual cl_int SetUp(const char *kernel_source)
Copy link
Contributor

Choose a reason for hiding this comment

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

Something doesn't seem right here - we have a kernel_source member variables and a kernel_source function argument. Do we need both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, param renamed

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.

Missing tests for work-item functions with out-of-range arguments
3 participants