Skip to content

[ESIMD] Allow constructing simd_view from simd #4514

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.

Sign up for GitHub

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 8 commits into from
Nov 2, 2021
Merged

[ESIMD] Allow constructing simd_view from simd #4514

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ca60c5b
[ESIMD] Allow constructing simd_view from simd
DenisBakhvalov Sep 8, 2021
d58dbd2
fixed code review comments
DenisBakhvalov Sep 16, 2021
6d40bc6
Merge branch 'sycl' into simd_view_ctor
kbobrovs Oct 19, 2021
c50f9c0
Merge remote-tracking branch 'intel_llvm/sycl' into simd_view_ctor
v-klochkov Nov 1, 2021
7a99aa0
Fix the LIT test; fix clang-format
v-klochkov Nov 1, 2021
8dfa127
Merge remote-tracking branch 'intel_llvm/sycl' into simd_view_ctor
v-klochkov Nov 2, 2021
33eede7
Also support simd_view constructor from 1 element simd
v-klochkov Nov 2, 2021
b4d1d4e
Address reviewer's comment - convert a comment to doxygen style
v-klochkov Nov 2, 2021
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
7 changes: 6 additions & 1 deletion sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ namespace detail {
/// It is an internal class implementing basic functionality of simd_view.
///
/// \ingroup sycl_esimd
template <typename BaseTy, typename RegionTy> class simd_view_impl {
template <typename BaseTy,
typename RegionTy =
region1d_t<typename BaseTy::element_type, BaseTy::length, 1>>
class simd_view_impl {
using Derived = simd_view<BaseTy, RegionTy>;
template <typename, int, class, class> friend class simd_obj_impl;
template <typename, int> friend class simd;
Expand Down Expand Up @@ -65,6 +68,8 @@ template <typename BaseTy, typename RegionTy> class simd_view_impl {
: M_base(Base), M_region(Region) {}
simd_view_impl(BaseTy &&Base, RegionTy Region)
: M_base(Base), M_region(Region) {}

simd_view_impl(BaseTy &Base) : M_base(Base), M_region(RegionTy(0)) {}
/// @}
public:
// Default copy and move constructors.
Expand Down
12 changes: 10 additions & 2 deletions sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ namespace esimd {
/// via an instance of this class.
///
/// \ingroup sycl_esimd
template <typename BaseTy, typename RegionTy>
template <typename BaseTy,
typename RegionTy =
region1d_t<typename BaseTy::element_type, BaseTy::length, 1>>
class simd_view : public detail::simd_view_impl<BaseTy, RegionTy> {
template <typename, int, class, class> friend class detail::simd_obj_impl;
template <typename, int> friend class detail::simd_mask_impl;
Expand Down Expand Up @@ -66,10 +68,13 @@ class simd_view : public detail::simd_view_impl<BaseTy, RegionTy> {
/// @}

public:
// Default copy and move constructors for simd_view.
/// Default copy and move constructors for simd_view.
simd_view(const simd_view &Other) = default;
simd_view(simd_view &&Other) = default;

/// Construct a complete view of a vector
simd_view(BaseTy &Base) : BaseClass(Base) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please convert this to a documentation comment, and mention that this does not work for VL=1

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. Is VL != 1 limitation still in effect? If so the best way would be to use SFINAE to disable this constructor in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I added constructor to scalar specialization. So, VL=1 case works. See this commit: 33eede7


simd_view &operator=(const simd_view &Other) {
BaseClass::operator=(Other);
return *this;
Expand Down Expand Up @@ -137,6 +142,9 @@ class simd_view<BaseTy, region1d_scalar_t<T>>
simd_view(BaseTy &&Base, RegionTy Region) : BaseClass(Base, Region) {}

public:
/// Construct a complete view of a vector
simd_view(BaseTy &Base) : BaseClass(Base) {}

operator element_type() const {
const auto v = BaseClass::read();
return v[0];
Expand Down
18 changes: 18 additions & 0 deletions sycl/test/esimd/simd_view.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,24 @@ SYCL_ESIMD_FUNCTION void test_simd_view_copy_ctor() {
auto v0_view_copy(v0_view);
}

// test construction from vector.
SYCL_ESIMD_FUNCTION void test_simd_view_from_vector() {
simd<int, 16> v16 = 0;
simd_view sv16a = v16;
simd_view sv16b(v16);
// expected-error@+5 {{no matching constructor for initialization of 'simd_view}}
// expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* 3 {{candidate }}
// expected-note@sycl/ext/intel/experimental/esimd/simd.hpp:* {{candidate }}
// expected-note@sycl/ext/intel/experimental/esimd/detail/simd_obj_impl.hpp:* {{candidate }}
// expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* 2 {{candidate }}
simd_view<simd<int, 16>, region_base<false, int, 1, 1, 16, 1>> sv16c(
(simd<int, 16>()));

simd<int, 1> v1 = 0;
simd_view sv1a = v1;
simd_view sv1b(v1);
}

// move constructor transfers the same view of the underlying data.
SYCL_ESIMD_FUNCTION void test_simd_view_move_ctor() {
simd<int, 16> v0 = 1;
Expand Down