[SYCL][COMPAT] Add math extend_v*4 to SYCLCompat #14078

OuadiElfarouki · 2024-06-06T13:03:13Z

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>

joeatodd

Hey @OuadiElfarouki nice work! Just a couple comments just now - to replace enable_if w/ static_assert. Apologies again for sending you the wrong way!

I notice the test math_extend_v.cpp is very slow (284 seconds when I run it locally). I'll investigate that further.

sycl/include/syclcompat/math.hpp

Alcpz

LGTM

OuadiElfarouki · 2024-06-07T11:07:11Z

@intel/llvm-gatekeepers PR is ready for merge 🙏🏻
Also CI memory_management.cpp failure is being tracked in Issue #14086
Thanks!

steffenlarsen · 2024-06-07T12:14:51Z

@joeatodd has requested a change to this PR and as such will need to approve before we can go ahead.

sycl/include/syclcompat/math.hpp

Co-authored-by: Yihan Wang <yihan.wang@intel.com>

sycl/include/syclcompat/math.hpp

yihanwg · 2024-06-11T08:28:45Z

sycl/include/syclcompat/math.hpp

+          typename BinaryOperation>
+inline constexpr RetT extend_vbinary4(AT a, BT b, RetT c,
+                                      BinaryOperation binary_op) {
+  static_assert(std::is_integral_v<AT> && std::is_integral_v<BT> &&


I think we can simplify to check whether AT and BT is int32_t/uint32_t, WDYT?

template <class T> constexpr inline bool is_i32_or_u32 = std::is_same_v<std::decay_t<T>, int32_t> || std::is_same_v<std::decay_t<T>, uint32_t>;

Yes that should look better @yihanwg. We don't want to put it inside the detail namespace though, any suggestion?

yihanwg · 2024-06-11T08:39:12Z

sycl/include/syclcompat/math.hpp

+    int16_t min_val = 0, max_val = 0;
+    min_val = std::numeric_limits<Tint>::min();
+    max_val = std::numeric_limits<Tint>::max();
+    temp = detail::clamp(temp, {min_val, min_val, min_val, min_val},


We can use the following code to construct a 4 element min_val/max_val vector.
https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#_vec_interface
explicit constexpr vec(const DataT& arg);

sycl::vec<int16_t, 4>(min_val), sycl::vec<int16_t, 4>(max_val)

Will apply for the v2 variant as well (just to keep it consistent, as the v2 extend got merged already).

joeatodd

LGTM

OuadiElfarouki · 2024-06-12T12:14:23Z

@intel/llvm-gatekeepers PR is ready! Thank you.

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>

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>

OuadiElfarouki and others added 16 commits May 29, 2024 19:43

[SYCL][COMPAT] add math extend_v*2 to SYCLCompat

2f908c4

Merge branch 'sycl' into math_extend_v2

f5b7e11

minor layout changes

3fa6edd

Update sycl/test-e2e/syclcompat/math/math_extend_v.cpp

033164e

Co-authored-by: Alberto Cabrera Pérez <alberto.cabrera@intel.com>

Fixed some gaps in test cases and other typo fixes

ca1f991

Explicit casting to extended uint type

a9d33b3

Minor fixes & reformatting

cd86f19

Extended some tests to cover mixed types

d65ab3b

fix to zero_or_signed_extend

065d920

Added extend_v2 APIs doc to readme

29ae741

Apply suggestions from code review

5e577bf

Co-authored-by: Joe Todd <joe.todd@codeplay.com>

fixed formatting

7018d4f

Merge branch 'sycl' into math_extend_v2

18c0163

Merge branch 'sycl' into math_extend_v2

d9d295c

[SYCL][COMPAT] add math extend_v*4 to SYCLCompat

cfa08ae

Added missing test

1e04a11

OuadiElfarouki requested a review from a team as a code owner June 6, 2024 13:03

OuadiElfarouki had a problem deploying to WindowsCILock June 6, 2024 13:13 — with GitHub Actions Error

OuadiElfarouki mentioned this pull request Jun 6, 2024

[SYCL][COMPAT] Add math extend_vcompare[2/4] to SYCLCompat #14079

Merged

Added extend_v4 APIs doc

f1de443

OuadiElfarouki had a problem deploying to WindowsCILock June 6, 2024 14:30 — with GitHub Actions Error

joeatodd requested changes Jun 6, 2024

View reviewed changes

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved

OuadiElfarouki added 3 commits June 6, 2024 16:47

replaced enable_if with static assertion for extend_vbinary2

bcd3698

Merge branch 'math_extend_v2' into math_extend_v4

00c7f16

replaced enable_if with static assertion for extend_vbinary4

00e97d1

OuadiElfarouki temporarily deployed to WindowsCILock June 6, 2024 16:11 — with GitHub Actions Inactive

OuadiElfarouki temporarily deployed to WindowsCILock June 6, 2024 18:15 — with GitHub Actions Inactive

Alcpz approved these changes Jun 7, 2024

View reviewed changes

Merge branch 'sycl' into math_extend_v4

8594248

OuadiElfarouki temporarily deployed to WindowsCILock June 7, 2024 16:19 — with GitHub Actions Inactive

OuadiElfarouki temporarily deployed to WindowsCILock June 7, 2024 17:53 — with GitHub Actions Inactive

OuadiElfarouki added 2 commits June 10, 2024 10:38

typo fix

6f0e5a0

Merge branch 'sycl' into math_extend_v4

d78dfc8

OuadiElfarouki temporarily deployed to WindowsCILock June 10, 2024 09:46 — with GitHub Actions Inactive

OuadiElfarouki temporarily deployed to WindowsCILock June 10, 2024 10:23 — with GitHub Actions Inactive

yihanwg reviewed Jun 11, 2024

View reviewed changes

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved

tangjj11 approved these changes Jun 11, 2024

View reviewed changes

yihanwg reviewed Jun 11, 2024

View reviewed changes

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved

Update sycl/include/syclcompat/math.hpp

2105e49

Co-authored-by: Yihan Wang <yihan.wang@intel.com>

OuadiElfarouki had a problem deploying to WindowsCILock June 11, 2024 08:06 — with GitHub Actions Error

yihanwg reviewed Jun 11, 2024

View reviewed changes

sycl/include/syclcompat/math.hpp Outdated Show resolved Hide resolved

Addressed PR comments

92be153

OuadiElfarouki temporarily deployed to WindowsCILock June 11, 2024 08:25 — with GitHub Actions Inactive

yihanwg reviewed Jun 11, 2024

View reviewed changes

OuadiElfarouki temporarily deployed to WindowsCILock June 11, 2024 08:48 — with GitHub Actions Inactive

tidy values concat using sycl::vec

274363d

OuadiElfarouki requested review from joeatodd and yihanwg June 12, 2024 07:52

OuadiElfarouki added 2 commits June 12, 2024 09:41

Simplified int/uint static type check

5e6ad05

Merge branch 'sycl' into math_extend_v4

9a538ad

OuadiElfarouki temporarily deployed to WindowsCILock June 12, 2024 08:48 — with GitHub Actions Inactive

OuadiElfarouki temporarily deployed to WindowsCILock June 12, 2024 09:16 — with GitHub Actions Inactive

joeatodd approved these changes Jun 12, 2024

View reviewed changes

steffenlarsen merged commit da735fe into intel:sycl Jun 12, 2024
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][COMPAT] Add math extend_v*4 to SYCLCompat #14078

[SYCL][COMPAT] Add math extend_v*4 to SYCLCompat #14078

OuadiElfarouki commented Jun 6, 2024

joeatodd left a comment

Alcpz left a comment

OuadiElfarouki commented Jun 7, 2024

steffenlarsen commented Jun 7, 2024

yihanwg Jun 11, 2024

OuadiElfarouki Jun 11, 2024

yihanwg Jun 11, 2024

OuadiElfarouki Jun 11, 2024

joeatodd left a comment

OuadiElfarouki commented Jun 12, 2024

[SYCL][COMPAT] Add math extend_v*4 to SYCLCompat #14078

[SYCL][COMPAT] Add math extend_v*4 to SYCLCompat #14078

Conversation

OuadiElfarouki commented Jun 6, 2024

joeatodd left a comment

Choose a reason for hiding this comment

Alcpz left a comment

Choose a reason for hiding this comment

OuadiElfarouki commented Jun 7, 2024

steffenlarsen commented Jun 7, 2024

yihanwg Jun 11, 2024

Choose a reason for hiding this comment

OuadiElfarouki Jun 11, 2024

Choose a reason for hiding this comment

yihanwg Jun 11, 2024

Choose a reason for hiding this comment

OuadiElfarouki Jun 11, 2024

Choose a reason for hiding this comment

joeatodd left a comment

Choose a reason for hiding this comment

OuadiElfarouki commented Jun 12, 2024