Skip to content

[SYCL][ESIMD] Convert esimd operator to friend method. #2994

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

Merged
merged 1 commit into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 24 additions & 23 deletions sycl/include/CL/sycl/INTEL/esimd/esimd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,30 +184,26 @@ template <typename Ty, int N> class simd {
// {/quote}
// * if not different, then auto should not be used
#define DEF_BINOP(BINOP, OPASSIGN) \
auto operator BINOP(const simd &RHS) const { \
ESIMD_INLINE friend auto operator BINOP(const simd &X, const simd &Y) { \
using ComputeTy = compute_type_t<simd>; \
auto V0 = convert<typename ComputeTy::vector_type>(data()); \
auto V1 = convert<typename ComputeTy::vector_type>(RHS.data()); \
auto V0 = convert<typename ComputeTy::vector_type>(X.data()); \
auto V1 = convert<typename ComputeTy::vector_type>(Y.data()); \
auto V2 = V0 BINOP V1; \
return ComputeTy(V2); \
} \
simd &operator OPASSIGN(const simd &RHS) { \
ESIMD_INLINE friend simd &operator OPASSIGN(simd &LHS, const simd &RHS) { \
using ComputeTy = compute_type_t<simd>; \
auto V0 = convert<typename ComputeTy::vector_type>(data()); \
auto V0 = convert<typename ComputeTy::vector_type>(LHS.data()); \
auto V1 = convert<typename ComputeTy::vector_type>(RHS.data()); \
auto V2 = V0 BINOP V1; \
write(convert<vector_type>(V2)); \
return *this; \
LHS.write(convert<vector_type>(V2)); \
return LHS; \
} \
simd &operator OPASSIGN(const Ty &RHS) { return *this OPASSIGN simd(RHS); }
ESIMD_INLINE friend simd &operator OPASSIGN(simd &LHS, const Ty &RHS) { \
LHS OPASSIGN simd(RHS); \
return LHS; \
}

// TODO @keryell
// {quote}
// Nowadays hidden friends seem to be more fashionable for these kind of
// operations. A nice side effect is that you have easily some scalar
// broadcast either on LHS & RHS.
// {/quote}
// TODO @mattkretz +1, ditto for compares
DEF_BINOP(+, +=)
DEF_BINOP(-, -=)
DEF_BINOP(*, *=)
Expand All @@ -222,8 +218,9 @@ template <typename Ty, int N> class simd {
// the future revisions.
//
#define DEF_RELOP(RELOP) \
simd<uint16_t, N> operator RELOP(const simd &RHS) const { \
auto R = data() RELOP RHS.data(); \
ESIMD_INLINE friend simd<uint16_t, N> operator RELOP(const simd &X, \
const simd &Y) { \
auto R = X.data() RELOP Y.data(); \
mask_type_t<N> M(1); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (maybe for another patch): since M is all ones, isn't M & convert<mask_type_t<N>>(R) equivalent to convert<mask_type_t<N>>(R) thus making M redundant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

M(1) is not part of my change. Not sure why it is written this way. I would be cautious to make unnecessary change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I see that it is not part of this change, that's why it is just a nit for another patch. I thought you wrote this part.

return M & convert<mask_type_t<N>>(R); \
}
Expand All @@ -238,16 +235,20 @@ template <typename Ty, int N> class simd {
#undef DEF_RELOP

#define DEF_LOGIC_OP(LOGIC_OP, OPASSIGN) \
simd operator LOGIC_OP(const simd &RHS) const { \
ESIMD_INLINE friend simd operator LOGIC_OP(const simd &X, const simd &Y) { \
static_assert(std::is_integral<Ty>(), "not integeral type"); \
auto V2 = data() LOGIC_OP RHS.data(); \
auto V2 = X.data() LOGIC_OP Y.data(); \
return simd(V2); \
} \
simd &operator OPASSIGN(const simd &RHS) { \
ESIMD_INLINE friend simd &operator OPASSIGN(simd &LHS, const simd &RHS) { \
static_assert(std::is_integral<Ty>(), "not integeral type"); \
auto V2 = data() LOGIC_OP RHS.data(); \
write(convert<vector_type>(V2)); \
return *this; \
auto V2 = LHS.data() LOGIC_OP RHS.data(); \
LHS.write(convert<vector_type>(V2)); \
return LHS; \
} \
ESIMD_INLINE friend simd &operator OPASSIGN(simd &LHS, const Ty &RHS) { \
LHS OPASSIGN simd(RHS); \
return LHS; \
}

DEF_LOGIC_OP(&, &=)
Expand Down
62 changes: 51 additions & 11 deletions sycl/include/CL/sycl/INTEL/esimd/esimd_view.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -180,13 +180,26 @@ template <typename BaseTy, typename RegionTy> class simd_view {
}

#define DEF_BINOP(BINOP, OPASSIGN) \
auto operator BINOP(const value_type &RHS) const { \
ESIMD_INLINE friend auto operator BINOP(const simd_view &X, \
const value_type &Y) { \
using ComputeTy = compute_type_t<value_type>; \
auto V0 = convert<typename ComputeTy::vector_type>(read().data()); \
auto V1 = convert<typename ComputeTy::vector_type>(RHS.data()); \
auto V0 = convert<typename ComputeTy::vector_type>(X.read().data()); \
auto V1 = convert<typename ComputeTy::vector_type>(Y.data()); \
auto V2 = V0 BINOP V1; \
return ComputeTy(V2); \
} \
ESIMD_INLINE friend auto operator BINOP(const value_type &X, \
const simd_view &Y) { \
using ComputeTy = compute_type_t<value_type>; \
auto V0 = convert<typename ComputeTy::vector_type>(X.data()); \
auto V1 = convert<typename ComputeTy::vector_type>(Y.read().data()); \
auto V2 = V0 BINOP V1; \
return ComputeTy(V2); \
} \
ESIMD_INLINE friend auto operator BINOP(const simd_view &X, \
const simd_view &Y) { \
return (X BINOP Y.read()); \
} \
simd_view &operator OPASSIGN(const value_type &RHS) { \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is value_type BINOP value_type combination missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot have that, cause ambiguous with the friend method definition in simd class

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see thanks.

using ComputeTy = compute_type_t<value_type>; \
auto V0 = convert<typename ComputeTy::vector_type>(read().data()); \
Expand All @@ -195,6 +208,9 @@ template <typename BaseTy, typename RegionTy> class simd_view {
auto V3 = convert<vector_type>(V2); \
write(V3); \
return *this; \
} \
simd_view &operator OPASSIGN(const simd_view &RHS) { \
return (*this OPASSIGN RHS.read()); \
}

DEF_BINOP(+, +=)
Expand All @@ -205,10 +221,21 @@ template <typename BaseTy, typename RegionTy> class simd_view {
#undef DEF_BINOP

#define DEF_RELOP(RELOP) \
simd<uint16_t, length> operator RELOP(const simd_view &RHS) const { \
auto R = read().data() RELOP RHS.read().data(); \
ESIMD_INLINE friend simd<uint16_t, length> operator RELOP( \
const simd_view &X, const value_type &Y) { \
auto R = X.read().data() RELOP Y.data(); \
mask_type_t<length> M(1); \
return M & convert<mask_type_t<length>>(R); \
} \
ESIMD_INLINE friend simd<uint16_t, length> operator RELOP( \
const value_type &X, const simd_view &Y) { \
auto R = X.data() RELOP Y.read().data(); \
mask_type_t<length> M(1); \
return M & convert<mask_type_t<length>>(R); \
} \
ESIMD_INLINE friend simd<uint16_t, length> operator RELOP( \
const simd_view &X, const simd_view &Y) { \
return (X RELOP Y.read()); \
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is value_type RELOP value_type combination missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot have that, cause ambiguous with the friend method definition in simd class


DEF_RELOP(>)
Expand All @@ -221,19 +248,32 @@ template <typename BaseTy, typename RegionTy> class simd_view {
#undef DEF_RELOP

#define DEF_LOGIC_OP(LOGIC_OP, OPASSIGN) \
simd_view operator LOGIC_OP(const simd_view &RHS) const { \
ESIMD_INLINE friend auto operator LOGIC_OP(const simd_view &X, \
const value_type &Y) { \
static_assert(std::is_integral<element_type>(), "not integral type"); \
auto V2 = read().data() LOGIC_OP RHS.read().data(); \
return simd_view(V2); \
auto V2 = X.read().data() LOGIC_OP Y.data(); \
return simd<element_type, length>(V2); \
} \
simd_view &operator OPASSIGN(const simd_view &RHS) { \
ESIMD_INLINE friend auto operator LOGIC_OP(const value_type &X, \
const simd_view &Y) { \
static_assert(std::is_integral<element_type>(), "not integral type"); \
auto V2 = X.data() LOGIC_OP Y.read().data(); \
return simd<element_type, length>(V2); \
} \
ESIMD_INLINE friend auto operator LOGIC_OP(const simd_view &X, \
const simd_view &Y) { \
return (X LOGIC_OP Y.read()); \
} \
simd_view &operator OPASSIGN(const value_type &RHS) { \
static_assert(std::is_integral<element_type>(), "not integeral type"); \
auto V2 = read().data LOGIC_OP RHS.read().data(); \
auto V2 = read().data() LOGIC_OP RHS.data(); \
auto V3 = convert<vector_type>(V2); \
write(V3); \
return *this; \
} \
simd_view &operator OPASSIGN(const simd_view &RHS) { \
return (*this OPASSIGN RHS.read()); \
}

DEF_LOGIC_OP(&, &=)
DEF_LOGIC_OP(|, |=)
DEF_LOGIC_OP(^, ^=)
Expand Down
2 changes: 1 addition & 1 deletion sycl/test/esimd/simd.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ bool test_simd_bin_ops() __attribute__((sycl_device)) {
simd<int, 8> v0 = 1;
simd<int, 8> v1 = 2;
v0 += v1;
v0 += 2;
v0 = 2 - v0;
v0 -= v1;
v0 -= 2;
v0 *= v1;
Expand Down
12 changes: 12 additions & 0 deletions sycl/test/esimd/simd_view.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,15 @@ bool test_simd_view_assign2() __attribute__((sycl_device)) {
v0.select<8, 1>(0) = v1.select<8, 1>(0);
return v0[0] == 1;
}

bool test_simd_view_assign3() __attribute__((sycl_device)) {
simd<int, 64> v0 = 0;
simd<int, 64> v1 = 1;
auto mask = (v0.select<16, 1>(0) > v1.select<16, 1>(0));
auto mask2 = (v0 > v1);
simd<ushort, 64> s = 0;
auto g4 = s.format<ushort, 4, 16>();
simd<ushort, 16> val = (g4.row(2) & mask);
simd<ushort, 16> val1 = (g4.row(2) & mask2.format<ushort, 4, 16>().row(0));
return val[0] == 0 && val1[0] == 0;
}