Skip to content

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.

Copy link
Contributor

@joeatodd joeatodd 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 @zhiweij1

@joeatodd
Copy link
Contributor

joeatodd commented Jan 9, 2025

@intel/llvm-gatekeepers this is ready to merge 🙏 CI failures unrelated.

@steffenlarsen steffenlarsen merged commit 6e0d90e into intel:sycl Jan 9, 2025
16 of 18 checks passed
@zhiweij1 zhiweij1 deleted the fix_syclcompat_math branch January 13, 2025 06:30
KornevNikita pushed a commit that referenced this pull request Feb 25, 2025
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