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

Missing error for wrong slice_pitch in clEnqueueReadImage #624

Closed
johnplate opened this issue Jun 13, 2021 · 3 comments · Fixed by #630
Closed

Missing error for wrong slice_pitch in clEnqueueReadImage #624

johnplate opened this issue Jun 13, 2021 · 3 comments · Fixed by #630
Assignees
Labels
needs-cts-coverage OpenCL API Spec

Comments

@johnplate
Copy link
Contributor

@johnplate johnplate commented Jun 13, 2021

The OpenCL spec lists the following requirement for the function clEnqueueReadImage:
"slice_pitch in clEnqueueReadImage and input_slice_pitch in clEnqueueWriteImage is the size in bytes of the 2D slice of the 3D region of a 3D image or each image of a 1D or 2D image array being read or written respectively. This must be 0 if image is a 1D or 2D image. [...]"

But slice_pitch is not mentioned in the list of returned errors for these functions. What should a CL implementation do if slice_pitch is not 0 for a 1D or 2D image? Return CL_INVALID_VALUE? Or ignore the error and return CL_SUCCESS?

If CL_INVALID_VALUE is returned, the CTS test mem_host_flags fails ...

@bashbaug bashbaug added this to To do in OpenCL specification maintenance via automation Jun 15, 2021
@bashbaug bashbaug added the OpenCL API Spec label Jun 15, 2021
@bashbaug bashbaug moved this from To do to Next Maintenance Release in OpenCL specification maintenance Jun 15, 2021
@alycm
Copy link
Contributor

@alycm alycm commented Jun 16, 2021

CL_INVALID_VALUE would seem reasonable to me.

Our implementation does return CL_INVALID_VALUE for this case. I just tested with an up-to-date CTS and we pass mem_host_flags, but perhaps the failure you see is connected to an optional type that we don't support.

However, the Intel GPU OpenCL implementation (which happens to be easy to run on my laptop) returns CL_SUCCESS.

@bashbaug
Copy link
Contributor

@bashbaug bashbaug commented Jun 16, 2021

the Intel GPU OpenCL implementation (which happens to be easy to run on my laptop) returns CL_SUCCESS.

Yes, we don't seem to be checking for this condition in our CPU or GPU implementation.

I agree that CL_INVALID_VALUE seems reasonable and this is returned by at least one other implementation I tested.

@johnplate
Copy link
Contributor Author

@johnplate johnplate commented Jun 16, 2021

I found the root cause why the test failed for me. It gets the slice pitch from an earlier clEnqueueMapImage, which should
(but doesn't) return zero for a 1D image. I have reported that bug to NVIDIA. They didn't notice the bug so far, because they ignore the non-zero slice pitch in clEnqueueReadImage, so the test succeeds for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cts-coverage OpenCL API Spec
Development

Successfully merging a pull request may close this issue.

3 participants