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

Remove usage of std::memcmp in bit_cast_test.h #1034

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

HPS-1
Copy link
Contributor

@HPS-1 HPS-1 commented Feb 7, 2025

Currently, test sycl_cts/bit_cast_test_core uses std::memcmp which is not guaranteed to be supported under SYCL 2020. Thus replacing it with a helper function that implement the functionality of std::memcmp without linking external library

@CLAassistant
Copy link

CLAassistant commented Feb 8, 2025

CLA assistant check
All committers have signed the CLA.

@HPS-1 HPS-1 marked this pull request as ready for review February 8, 2025 10:53
@HPS-1 HPS-1 requested a review from a team as a code owner February 8, 2025 10:53
@bader
Copy link
Contributor

bader commented Feb 8, 2025

Thus replacing it with memcmp among sycl built-in functions.

I can't find memcmp in SYCL 2020 spec neither. Are you sure that dropping std:: namespace replaces std version with SYCL and not C version? I would add sycl:: to validate that, but assuming that SYCL 2020 doesn't provide memcmp device function this should not work either.

Probably the easiest way is to implement memcpy w/o using external libraries as SYCL-CTS utility function.

@HPS-1
Copy link
Contributor Author

HPS-1 commented Feb 10, 2025

Thus replacing it with memcmp among sycl built-in functions.

I can't find memcmp in SYCL 2020 spec neither. Are you sure that dropping std:: namespace replaces std version with SYCL and not C version? I would add sycl:: to validate that, but assuming that SYCL 2020 doesn't provide memcmp device function this should not work either.

Probably the easiest way is to implement memcpy w/o using external libraries as SYCL-CTS utility function.

Hi Alexey! Thanks for reviewing this. I've fixed the issue as you suggested. Could you please take a look and let me know if it looks good? Thanks! @bader

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