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

Is calling clSetKernelArg with an arg_size of 0 valid for arguments in the local address space? #1317

Open
kpet opened this issue Feb 17, 2025 · 2 comments · May be fixed by #1319
Open

Is calling clSetKernelArg with an arg_size of 0 valid for arguments in the local address space? #1317

kpet opened this issue Feb 17, 2025 · 2 comments · May be fixed by #1319
Labels
needs-cts-coverage The CTS needs to be extended

Comments

@kpet
Copy link
Contributor

kpet commented Feb 17, 2025

Some implementations accept calls to clSetKernelArg with an arg_size of 0 for arguments in the local address space, others return CL_INVALID_ARG_SIZE.

Should this be allowed? I don't see why it should not. We should also use this discussion as an opportunity to check that we have language that correctly describes what memory accesses are valid for kernel arguments in the local address space.

Lastly, regardless of the decision, we need a new CTS test.

@kpet kpet added the needs-cts-coverage The CTS needs to be extended label Feb 17, 2025
@karolherbst
Copy link
Contributor

spec says it should error:

CL_INVALID_ARG_SIZE [...] if arg_size is zero and the argument is declared with the local qualifier

kpet added a commit to kpet/OpenCL-Docs that referenced this issue Feb 18, 2025
This would likely have prevented me from creating KhronosGroup#1317 after
reading the spec a bit too quickly.

Fixes KhronosGroup#1317

Signed-off-by: Kevin Petit <kevin.petit@arm.com>
Change-Id: I1ed3737335edc07747e872821210e5785ae6635a
@kpet
Copy link
Contributor Author

kpet commented Feb 18, 2025

As discussed in the 2025/02/18 teleconference, I missed this when reading the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cts-coverage The CTS needs to be extended
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants