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

Query out and use local size set in program IL in CL adapter. #2160

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

Conversation

aarongreig
Copy link
Contributor

The CL spec wording on this is kind of fuzzy but every CL driver I tested (across intel, amd, nvidia cpu + gpu) returns an error when you have a local size set in the program source/IL and you don't specify any local size in your clEnqueueNDRangeKernel call (i.e. you leave it as NULL).

Our spec does allow you to leave local size as null if you have a size specified in your program, so this change adds some logic to query out the size set in the program and passes it to the enqueue call.

Initially I was concerned this might impact performance of current users but it looks like SYCL always passes a local size when calling urEnqueueKernelLaunch so it won't hit the path with the extra query.

The CL spec wording on this is kind of fuzzy but every CL driver I
tested (across intel, amd, nvidia cpu + gpu) returns an error when you
have a local size set in the program source/IL and you don't specify any
local size in your clEnqueueNDRangeKernel call (i.e. you leave it as
NULL).

Our spec does allow you to leave local size as null if you have a size
specified in your program, so this change adds some logic to query out
the size set in the program and passes it to the enqueue call.

Initially I was concerned this might impact performance of current users
but it looks like SYCL always passes a local size when calling
urEnqueueKernelLaunch so it won't hit the path with the extra query.
@github-actions github-actions bot added the conformance Conformance test suite issues. label Oct 1, 2024
@aarongreig
Copy link
Contributor Author

LLVM PR intel/llvm#15571

@aarongreig aarongreig marked this pull request as ready for review October 2, 2024 09:19
@aarongreig aarongreig requested review from a team as code owners October 2, 2024 09:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conformance Conformance test suite issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants