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

[SYCLCompat] Fix vectorized_binary impl to make SYCLomatic migrated code run pass #16553

Open
wants to merge 7 commits into
base: sycl
Choose a base branch
from

Conversation

zhiweij1
Copy link
Contributor

@zhiweij1 zhiweij1 commented Jan 8, 2025

Signed-off-by: Jiang, Zhiwei [email protected]

@zhiweij1 zhiweij1 requested a review from a team as a code owner January 8, 2025 03:26
Signed-off-by: Jiang, Zhiwei <[email protected]>
Signed-off-by: Jiang, Zhiwei <[email protected]>
Signed-off-by: Jiang, Zhiwei <[email protected]>
Signed-off-by: Jiang, Zhiwei <[email protected]>
Signed-off-by: Jiang, Zhiwei <[email protected]>
@joeatodd
Copy link
Contributor

joeatodd commented Jan 8, 2025

Hey @zhiweij1 looks good! I understand now that you are altering the semantics of vectorized_binary for comparison operators by specifying the template argument (e.g. std::equal_to<unsigned short> vs std::equal_to<>). I think we misunderstood this previously. Can I suggest the following addition to sycl/doc/syclcompat/README.md after line 2091:

`vectorized_binary` also supports comparison operators from the standard library (`std::equal_to`, `std::not_equal_to`, etc) 
and the semantics can be modified by changing the comparison operator template instantiation. For example:

```cpp
unsigned int Input1;
unsigned int Input2;
// initialize inputs...

// Performs comparison on sycl::ushort2, following sycl::vec semantics
// Returns unsigned int containing, per vector element, 0xFFFF if true, and 0x0000 if false
syclcompat::vectorized_binary<sycl::ushort2>(
      Input1, Input2, std::equal_to<>());

// Performs element-wise comparison on unsigned short
// Returns unsigned int containing, per vector element, 1 if true, and 0 if false
syclcompat::vectorized_binary<sycl::ushort2>(
      Input1, Input2, std::equal_to<unsigned short>());

Signed-off-by: Jiang, Zhiwei <[email protected]>
@zhiweij1
Copy link
Contributor Author

zhiweij1 commented Jan 8, 2025

Hey @zhiweij1 looks good! I understand now that you are altering the semantics of vectorized_binary for comparison operators by specifying the template argument (e.g. std::equal_to<unsigned short> vs std::equal_to<>). I think we misunderstood this previously. Can I suggest the following addition to sycl/doc/syclcompat/README.md after line 2091:

`vectorized_binary` also supports comparison operators from the standard library (`std::equal_to`, `std::not_equal_to`, etc) 
and the semantics can be modified by changing the comparison operator template instantiation. For example:

```cpp
unsigned int Input1;
unsigned int Input2;
// initialize inputs...

// Performs comparison on sycl::ushort2, following sycl::vec semantics
// Returns unsigned int containing, per vector element, 0xFFFF if true, and 0x0000 if false
syclcompat::vectorized_binary<sycl::ushort2>(
      Input1, Input2, std::equal_to<>());

// Performs element-wise comparison on unsigned short
// Returns unsigned int containing, per vector element, 1 if true, and 0 if false
syclcompat::vectorized_binary<sycl::ushort2>(
      Input1, Input2, std::equal_to<unsigned short>());

Done in c7b27d3

@zhiweij1
Copy link
Contributor Author

zhiweij1 commented Jan 9, 2025

@joeatodd Before adjusting the doc file, the CI has passed. So I think the failure in the latest CI is not related to 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.

2 participants