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

[tests] Fix for permutation_iterator tests with transform_iterator index #1489

Merged

Conversation

danhoeflinger
Copy link
Contributor

@danhoeflinger danhoeflinger commented Apr 9, 2024

This PR should fix the permutation_iterator tests with transform_iterator as index. In practice, without this PR, errors only appear on Windows platforms, however it may not be limited to windows.

The SYCL spec requires that template arguments (captures) to lambda functions submitted as sycl kernels must be included in the kernel name and must be "forward declarable at namespace scope".
https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:interfaces.kernels.as.lambdas

When we use a functor in a transform_iterator which is then used as index for a permutation_iterator, the type of the resulting range must be forward declarable at namespace scope as it is used in the reduce kernel as an input capture to the lambda sycl kernel. When we define the functor within a function's scope, rather than within the structure's scope, we lose this, and end up with an invalid kernel name. This PR simply moves the functor definition to a scope which makes it forward declarable at namespace scope.

On windows, it seems that this results in an exception when attempting to call the kernel:
"No kernel named was found -46 (PI_ERROR_INVALID_KERNEL_NAME)"

Copy link
Contributor

@timmiesmith timmiesmith left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the explanation.

@danhoeflinger danhoeflinger marked this pull request as ready for review April 9, 2024 16:43
Copy link
Contributor

@SergeyKopienko SergeyKopienko left a comment

Choose a reason for hiding this comment

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

LGTM

@danhoeflinger danhoeflinger merged commit acfdf36 into main Apr 10, 2024
20 checks passed
@danhoeflinger danhoeflinger deleted the dev/dhoeflin/fix_permutation_iter_transform_iter_idx_tests branch April 10, 2024 13:26
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