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

[SYCL][COMPAT] Add math extend_v*2 to SYCLCompat #13953

Merged
merged 15 commits into from
Jun 7, 2024

Conversation

OuadiElfarouki
Copy link
Contributor

This PR adds math extend_v*2 operators (18 in total) along with unit-tests for signed and unsigned int32 cases.

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.

Thanks for this @OuadiElfarouki! I left a few comments which largely follow the discussion we've been having in the team.

I think we need to decide what we want to do about permitting mixed signed/unsigned types. Exhaustively testing every combination is probably intractable, but we could test all 8 permutations of signed/unsigned, signed/unsigned, signed/unsigned for at least one or two of the extend_v*2 APIs.

@Alcpz what are your thoughts on that?

sycl/test-e2e/syclcompat/math/math_extend_v.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/syclcompat/math/math_extend_v.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/syclcompat/math/math_extend_v.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/syclcompat/math/math_extend_v.cpp Outdated Show resolved Hide resolved
sycl/test-e2e/syclcompat/math/math_extend_v.cpp Outdated Show resolved Hide resolved
sycl/include/syclcompat/math.hpp Show resolved Hide resolved
sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved
sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved
@joeatodd
Copy link
Contributor

We'll also need to add the APIs to the README, but let's wait till we resolve the rest of the stuff to avoid duplication of effort.

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>
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.

Looks good 👍

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved
sycl/include/syclcompat/math.hpp Show resolved Hide resolved
sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved
Co-authored-by: Joe Todd <joe.todd@codeplay.com>
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.

🚢

@OuadiElfarouki
Copy link
Contributor Author

@intel/llvm-gatekeepers PR is ready for merge. (The #14078 can be addressed later) 🙏🏻

@martygrant martygrant merged commit 572aa5c into intel:sycl Jun 7, 2024
13 of 14 checks passed
steffenlarsen pushed a commit that referenced this pull request Jun 12, 2024
This PR adds math `extend_v*4` operators (18 in total) along with
unit-tests for signed and unsigned int32 cases.
*Some changes overlap with the previous `extend_v*2` PR #13953 and thus
should be reviewed/merged first.

---------

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>
Co-authored-by: Joe Todd <joe.todd@codeplay.com>
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
This PR adds math `extend_v*2` operators _(18 in total)_ along with
unit-tests for signed and unsigned `int32` cases.

---------

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>
Co-authored-by: Joe Todd <joe.todd@codeplay.com>
ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
This PR adds math `extend_v*4` operators (18 in total) along with
unit-tests for signed and unsigned int32 cases.
*Some changes overlap with the previous `extend_v*2` PR intel#13953 and thus
should be reviewed/merged first.

---------

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>
Co-authored-by: Joe Todd <joe.todd@codeplay.com>
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
steffenlarsen pushed a commit that referenced this pull request Jun 14, 2024
This PR adds math `extend_vcompare[2/4] `operators (4 in total) along
with unit-tests for signed and unsigned int32 cases.
Also, Unit-tests from previous `extend_v*4` #14078 and `extend_v*2`
#13953 are moved to two different files.

---------

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>
Co-authored-by: Joe Todd <joe.todd@codeplay.com>
Co-authored-by: Yihan Wang <yihan.wang@intel.com>
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.

4 participants