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

[ESIMD] Add esimd::merge free function. #5308

Merged
merged 2 commits into from
Jan 19, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions sycl/include/sycl/ext/intel/experimental/esimd.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

/// \defgroup sycl_esimd DPC++ Explicit SIMD API

#include <sycl/ext/intel/experimental/esimd/alt_ui.hpp>
#include <sycl/ext/intel/experimental/esimd/common.hpp>
#include <sycl/ext/intel/experimental/esimd/math.hpp>
#include <sycl/ext/intel/experimental/esimd/memory.hpp>
Expand Down
61 changes: 61 additions & 0 deletions sycl/include/sycl/ext/intel/experimental/esimd/alt_ui.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//==-------------- alt_ui.hpp - DPC++ Explicit SIMD API ------------------==//
//
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
// See https://llvm.org/LICENSE.txt for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
// "Alternative" convenience Explicit SIMD APIs.
//===----------------------------------------------------------------------===//

#include <sycl/ext/intel/experimental/esimd/simd.hpp>
#include <sycl/ext/intel/experimental/esimd/simd_view.hpp>

__SYCL_INLINE_NAMESPACE(cl) {
namespace sycl {
namespace ext {
namespace intel {
namespace experimental {
namespace esimd {

/// "Merges" elements of the input vectors according to the merge mask.
/// @param a the first vector
/// @param b the second vector
/// @param m the merge mask
/// @return a vector, where each element equals to corresponding element from
/// \c a (if corresponding mask element is zero) or \c b (otherwise)
/// \ingroup sycl_esimd
template <class T, int N>
__ESIMD_API simd<T, N> merge(simd<T, N> a, simd<T, N> b, simd_mask<N> m) {
v-klochkov marked this conversation as resolved.
Show resolved Hide resolved
a.merge(b, m);
return a;
}

/// "Merges" elements of the input masks according to the merge mask.
template <int N>
__ESIMD_API simd_mask<N> merge(simd_mask<N> a, simd_mask<N> b, simd_mask<N> m) {
a.merge(b, m);
return a;
}

/// "Merges" elements of vectors referenced by the input views.
/// Available only when all of the following holds:
/// - the length and the element type of the sub-regions referenced by both
/// input views are the same.
template <class BaseT1, class BaseT2, class RegionT1, class RegionT2,
class = std::enable_if_t<
(shape_type<RegionT1>::length == shape_type<RegionT2>::length) &&
std::is_same_v<detail::element_type_t<BaseT1>,
detail::element_type_t<BaseT2>>>>
__ESIMD_API auto merge(simd_view<BaseT1, RegionT1> v1,
simd_view<BaseT2, RegionT2> v2,
simd_mask<shape_type<RegionT1>::length> m) {
return merge(v1.read(), v2.read(), m);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the usage of the mask here consistent with the other two overloads?

The first two use a.merge(b, m), and the documentation says "The semantic is that if the LSB of an element of Mask is set, then the corresponding data element of Val is copied to the corresponding position in the calling simd object." -- b is copied if m is true.

This one uses merge(a, b, m), and the documentation says "The semantic is that if the LSB of an element of Mask is set, then the corresponding data element of Val1 is copied to the corresponding position in the calling simd object".

Am I reading the documentation wrong, or should this be merge(b, a, m) to match the semantics?

(A simpler way to ask the same question might be: is the behavior of this two-input version of merge the same as the two-input member function?)

Copy link
Contributor Author

@kbobrovs kbobrovs Jan 15, 2022

Choose a reason for hiding this comment

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

Thanks for the catch of this discrepancy in the documentation, John. The actual behavior is that any non-zero in a mask element (e.g. 0x8) is considered as "enabled", so this version is correct, and the one is not. I'm working on updating the documentation, so I will address this.
The behavior is consistent across all versions/overloads of merge, as those ones are implemented through the other.

}

} // namespace esimd
} // namespace experimental
} // namespace intel
} // namespace ext
} // namespace sycl
} // __SYCL_INLINE_NAMESPACE(cl)
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ class simd_mask_impl
}

/// Implicit conversion from simd.
__SYCL_DEPRECATED(__ESIMD_MASK_DEPRECATION_MSG)
simd_mask_impl(const simd<T, N> &Val) : base_type(Val.data()) {}

/// Implicit conversion from simd_view<simd,...>.
Expand Down
50 changes: 50 additions & 0 deletions sycl/test/esimd/esimd_merge.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// RUN: %clangxx -fsycl -fsycl-device-only -fsyntax-only -Xclang -verify %s
// expected-no-diagnostics

#include <sycl/ext/intel/experimental/esimd.hpp>

using namespace sycl::ext::intel::experimental::esimd;
using namespace sycl::ext::intel::experimental;
using namespace cl::sycl;

using simd_mask_elem_t = simd_mask<1>::element_type;

template <class T, int N> struct SimdMergeTest {
inline void test(T *in_a, T *in_b, simd_mask_elem_t *in_mask, T *out)
__attribute__((sycl_device)) {
simd<T, N> a(in_a);
simd<T, N> b(in_b);
simd_mask<N> m(in_mask);
simd<T, N> c = esimd::merge(a, b, m);
c.copy_to(out);
}
};

template <class T, int N> struct SimdViewMergeTest {
inline void test(T *in_a, T *in_b, simd_mask_elem_t *in_mask, T *out)
__attribute__((sycl_device)) {
simd<T, N> a(in_a);
simd<T, N> b(in_b);
simd_mask<N / 2> m(in_mask);
simd<T, N / 2> c = esimd::merge(a.template select<N / 2, 1>(1),
b.template select<N / 2, 2>(0), m);
c.copy_to(out);
}
};

template <class T, int N> struct SimdView2MergeTest {
inline void test(T *in_a, T *in_b, simd_mask_elem_t *in_mask, T *out)
__attribute__((sycl_device)) {
simd<T, N> a(in_a);
simd<T, N> b(in_b);
simd_mask<N / 4> m(in_mask);
simd<T, N / 4> c = esimd::merge(
a.template select<N / 2, 1>(1).template select<N / 4, 1>(),
b.template select<N / 2, 2>(0).template select<N / 4, 1>(), m);
c.copy_to(out);
}
};

template struct SimdMergeTest<float, 8>;
template struct SimdViewMergeTest<float, 8>;
template struct SimdView2MergeTest<sycl::half, 8>;
4 changes: 0 additions & 4 deletions sycl/test/esimd/simd_mask.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,16 +63,12 @@ SYCL_EXTERNAL SYCL_ESIMD_FUNCTION void compat_test(float *ptr) {
simd<int, 8> pred1(1);
auto pred2 = pred1.bit_cast_view<unsigned short>();

// expected-warning@+1 {{deprecated}}
auto x1 = gather<float, 16>(ptr, offsets, pred);
// expected-warning@+1 {{deprecated}}
auto x11 = gather<float, 16>(ptr, offsets, pred2);
// expected-warning@+1 {{deprecated}}
auto x2 = gather<float, 16>(ptr, offsets, simd<unsigned short, 16>{});
simd_mask<16> m1(0);
// expected-warning@+1 {{deprecated}}
m1 = pred;
simd_mask<16> m2(0);
// expected-warning@+1 {{deprecated}}
m2 = std::move(pred);
}