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 bug in mutable_command_full_dispatch #2082

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

Conversation

gorazd-sumkovski-arm
Copy link
Contributor

This test did not pass the -cl-std= flag when building the program. As a result, for an OpenCL 3.0 device, the program will be "compiled using the highest OpenCL C 1.x language version supported" by the device. However this will force uniform work-group sizes which leads to a CL_INVALID_WORK_GROUP_SIZE error.

To fix this, use the create_single_kernel_helper() helper function which will automatically get the device version and pass that to -cl-std= when building the program.

This test did not pass the `-cl-std=` flag when building the program. As
a result, for an OpenCL 3.0 device, the program will be "compiled using
the highest OpenCL C 1.x language version supported" by the device.
However this will force uniform work-group sizes which leads to a
`CL_INVALID_WORK_GROUP_SIZE` error.

To fix this, use the `create_single_kernel_helper()` helper function
which will automatically get the device version and pass that to
`-cl-std=` when building the program.

Signed-off-by: Gorazd Sumkovski <gorazd.sumkovski@arm.com>
@bashbaug
Copy link
Contributor

I'm OK with this change and using the helper functions is good in general, but should we also update this test so it does not require non-uniform work-groups? I don't see any place where we check for non-uniform work-group support, so even when we compile for OpenCL C 3.0 it's possible it won't work if it runs on a device that does not support non-uniform work-groups.

FWIW also, when I run this test on our devices using the command buffer emulation layer I don't see use of non-uniform work-groups.

Copy link
Contributor

@EwanC EwanC left a comment

Choose a reason for hiding this comment

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

Change itself LGTM.

Agree with Ben's point that the test is assuming uniform workgroups, and we should either update the kernel to remove that assumption or check the device. Happy for this improvement to be filed as an issue if not wanting to be done in this PR.

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.

3 participants