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

Rng mklcpu backend uses ambiguous kernel names #89

Closed
sbalint98 opened this issue May 10, 2021 · 3 comments · Fixed by #90
Closed

Rng mklcpu backend uses ambiguous kernel names #89

sbalint98 opened this issue May 10, 2021 · 3 comments · Fixed by #90
Labels
accepted The issue/feature is confirmed and going to be implemented bug A request to fix an issue

Comments

@sbalint98
Copy link
Contributor

Summary

In the rng mklcpu backend in philox4x32x10.cpp the same kernel names are generated for both the USM and buffer API. This does not cause an error when using dpc++, but according to the SYCL standard, this is illegal and causes problems with some SYCL implementations, tested with hipSYCL.

Observed behavior

For example, the kernels at line 328 and 68 in philox4x32x10.cpp appear to have the same name. This is a result of the combination of the type of distr and philox4x32x10_impl being used as kernel names. Since kernels are declared for both USM and buffer interfaces with the same type of distr this results in ambiguous kernel names.

Expected behavior

All kernels should have a unique name.

@sbalint98 sbalint98 changed the title Rng mklcpu backend violates uses ambiguous kernel names Rng mklcpu backend uses ambiguous kernel names May 10, 2021
@aelizaro
Copy link
Contributor

Hi, @sbalint98, thank you for noticing this issue, I will fix it. Is this issue blocking you? If yes, I will do it as soon as possible.

@aelizaro aelizaro added accepted The issue/feature is confirmed and going to be implemented bug A request to fix an issue labels May 11, 2021
@sbalint98
Copy link
Contributor Author

Hi, @aelizaro thanks for the quick response. This issue is not critical for me.

@aelizaro
Copy link
Contributor

ok, thank you! Then I expect to fix it by the next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted The issue/feature is confirmed and going to be implemented bug A request to fix an issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants