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

fix profiling profiling_timebase #2241

Closed
wants to merge 1 commit into from

Conversation

rjodinchr
Copy link
Contributor

Use explicit work_group_size of 1

Fix #2240

Use explicit work_group_size of 1

Fix KhronosGroup#2240
Copy link
Contributor

@bashbaug bashbaug left a comment

Choose a reason for hiding this comment

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

I'm OK with this change since I don't think it's an important part of the test to have a global_work_size equal to NULL, but I do believe this part of the test is correct, and a global_work_size equal to NULL is valid - at least for OpenCL 2.1 and newer. I've filed a few issues to get this clarified, and to ensure that the case where the global_work_size is equal to NULL is tested:

KhronosGroup/OpenCL-Docs#1302
#2244

Note, clGetDeviceAndHostTimer also requires OpenCL 2.1, and there's no check for OpenCL 2.1 or newer (that I can see at least), so this really ought to be added. If you'd prefer to add this check in a separate PR, would you mind filing an issue so it doesn't slip through the cracks? I suspect it will be less work to just add the check, though 😄

@rjodinchr
Copy link
Contributor Author

Alright, I missed that part of the spec. Honestly having something about global_work_size being NULL not in the global_work_size section but the work_dim section is really easy to miss.

In that case, I think we should not land that PR. I'll fix clvk according to the spec.

@rjodinchr rjodinchr closed this Jan 23, 2025
@rjodinchr rjodinchr deleted the pr/profiling-timebase branch January 23, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

profiling profiling_timebase using NULL as global_work_size in clEnqueueNDRangeKernel
2 participants