From ca60c5b76bbb3e675fb21fd1a1f9aa47cccee394 Mon Sep 17 00:00:00 2001 From: Denis Bakhvalov Date: Tue, 7 Sep 2021 22:22:04 -0700 Subject: [PATCH 1/5] [ESIMD] Allow constructing simd_view from simd The patch allows the following code to work: ``` simd s = 0; simd_view v = s; ``` I added default template parameter for simd_view_impl class and put it as a last template argument. --- .../esimd/detail/simd_view_impl.hpp | 5 +++- .../intel/experimental/esimd/simd_view.hpp | 26 ++++++++++++------- sycl/test/esimd/simd_view.cpp | 6 +++++ 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp index 56c4e626118b8..ec9bddacc6c6d 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp @@ -25,7 +25,9 @@ namespace detail { /// It is an internal class implementing basic functionality of simd_view. /// /// \ingroup sycl_esimd -template +template > class simd_view_impl { template friend class simd; template friend class simd_view_impl; @@ -61,6 +63,7 @@ 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. diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp index 93772330450ca..0ab2ded19e2a2 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp @@ -24,15 +24,18 @@ namespace esimd { /// via an instance of this class. /// /// \ingroup sycl_esimd -template -class simd_view : public detail::simd_view_impl> { +template > +class simd_view + : public detail::simd_view_impl, + RegionTy> { template friend class simd; template friend class detail::simd_view_impl; public: using BaseClass = - detail::simd_view_impl>; + detail::simd_view_impl, RegionTy>; using ShapeTy = typename shape_type::type; static constexpr int length = ShapeTy::Size_x * ShapeTy::Size_y; @@ -41,6 +44,9 @@ class simd_view : public detail::simd_view_impl; + // Construct a complete view of a vector + simd_view(BaseTy &Base) : BaseClass(Base) {} + private: simd_view(BaseTy &Base, RegionTy Region) : BaseClass(Base, Region) {} simd_view(BaseTy &&Base, RegionTy Region) : BaseClass(Base, Region) {} @@ -173,15 +179,15 @@ class simd_view : public detail::simd_view_impl class simd_view> : public detail::simd_view_impl< - BaseTy, region1d_scalar_t, - simd_view>> { + BaseTy, simd_view>, + region1d_scalar_t> { template friend class simd; template friend class detail::simd_view_impl; public: using RegionTy = region1d_scalar_t; using BaseClass = - detail::simd_view_impl>; + detail::simd_view_impl, RegionTy>; using ShapeTy = typename shape_type::type; static constexpr int length = ShapeTy::Size_x * ShapeTy::Size_y; static_assert(1 == length, "length of this view is not equal to 1"); @@ -237,9 +243,9 @@ class simd_view, NestedRegion>> : public detail::simd_view_impl< BaseTy, - std::pair, NestedRegion>, simd_view, - NestedRegion>>> { + NestedRegion>>, + std::pair, NestedRegion>> { template friend class simd; template friend class detail::simd_view_impl; @@ -247,7 +253,7 @@ class simd_view, NestedRegion>; using BaseClass = - detail::simd_view_impl>; + detail::simd_view_impl, RegionTy>; using ShapeTy = typename shape_type::type; static constexpr int length = ShapeTy::Size_x * ShapeTy::Size_y; static_assert(1 == length, "length of this view is not equal to 1"); diff --git a/sycl/test/esimd/simd_view.cpp b/sycl/test/esimd/simd_view.cpp index d90eb600b27a0..79cad8756bb0f 100644 --- a/sycl/test/esimd/simd_view.cpp +++ b/sycl/test/esimd/simd_view.cpp @@ -69,6 +69,12 @@ 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 s = 0; + simd_view v = s; +} + // move constructor transfers the same view of the underlying data. SYCL_ESIMD_FUNCTION void test_simd_view_move_ctor() { simd v0 = 1; From d58dbd2cc3fa887e973a62647b99137a894093a3 Mon Sep 17 00:00:00 2001 From: Denis Bakhvalov Date: Wed, 15 Sep 2021 21:49:39 -0700 Subject: [PATCH 2/5] fixed code review comments --- sycl/test/esimd/simd_view.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/sycl/test/esimd/simd_view.cpp b/sycl/test/esimd/simd_view.cpp index 79cad8756bb0f..bc97506583be0 100644 --- a/sycl/test/esimd/simd_view.cpp +++ b/sycl/test/esimd/simd_view.cpp @@ -72,7 +72,14 @@ SYCL_ESIMD_FUNCTION void test_simd_view_copy_ctor() { // test construction from vector. SYCL_ESIMD_FUNCTION void test_simd_view_from_vector() { simd s = 0; - simd_view v = s; + simd_view v1 = s; + simd_view v2(s); + // expected-error@+4 {{no matching constructor for initialization of 'simd_view}} + // expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* {{candidate constructor not viable: expects an lvalue for 1st argument}} + // expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* 4+ {{candidate constructor not viable:}} + // expected-note@sycl/ext/intel/experimental/esimd/simd.hpp:* {{candidate template ignored:}} + simd_view, region_base> v3( + (simd())); } // move constructor transfers the same view of the underlying data. From 7a99aa036d4d6b06e34e4e2cd393b7de9d8cc5a8 Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Mon, 1 Nov 2021 15:10:47 -0700 Subject: [PATCH 3/5] Fix the LIT test; fix clang-format Signed-off-by: Vyacheslav N Klochkov --- .../experimental/esimd/detail/simd_view_impl.hpp | 3 ++- .../sycl/ext/intel/experimental/esimd/simd_view.hpp | 3 ++- sycl/test/esimd/simd_view.cpp | 11 ++++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp index 491225e4d9b03..d87f64e464909 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp @@ -25,7 +25,8 @@ namespace detail { /// It is an internal class implementing basic functionality of simd_view. /// /// \ingroup sycl_esimd -template > class simd_view_impl { using Derived = simd_view; diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp index 42aadebb6ee4b..c88ee4907d1b6 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp @@ -25,7 +25,8 @@ namespace esimd { /// via an instance of this class. /// /// \ingroup sycl_esimd -template > class simd_view : public detail::simd_view_impl { template friend class detail::simd_obj_impl; diff --git a/sycl/test/esimd/simd_view.cpp b/sycl/test/esimd/simd_view.cpp index 05c4c0aa1f55c..710f7bf2cb24f 100644 --- a/sycl/test/esimd/simd_view.cpp +++ b/sycl/test/esimd/simd_view.cpp @@ -111,11 +111,12 @@ SYCL_ESIMD_FUNCTION void test_simd_view_from_vector() { simd s = 0; simd_view v1 = s; simd_view v2(s); - // expected-error@+4 {{no matching constructor for initialization of 'simd_view}} - // expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* {{candidate constructor not viable: expects an lvalue for 1st argument}} - // expected-note@sycl/ext/intel/experimental/esimd/simd_view.hpp:* 4+ {{candidate constructor not viable:}} - // expected-note@sycl/ext/intel/experimental/esimd/simd.hpp:* {{candidate template ignored:}} - simd_view, region_base> v3( + // 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, region_base> v3( (simd())); } From 33eede79f2ca2d92d21dca4b85ca2227bebeb47d Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Tue, 2 Nov 2021 12:11:04 -0700 Subject: [PATCH 4/5] Also support simd_view constructor from 1 element simd Signed-off-by: Vyacheslav N Klochkov --- .../sycl/ext/intel/experimental/esimd/simd_view.hpp | 3 +++ sycl/test/esimd/simd_view.cpp | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp index c88ee4907d1b6..774d6c1e2715c 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp @@ -142,6 +142,9 @@ class simd_view> 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]; diff --git a/sycl/test/esimd/simd_view.cpp b/sycl/test/esimd/simd_view.cpp index 710f7bf2cb24f..602dd8fa48c20 100644 --- a/sycl/test/esimd/simd_view.cpp +++ b/sycl/test/esimd/simd_view.cpp @@ -108,16 +108,20 @@ SYCL_ESIMD_FUNCTION void test_simd_view_copy_ctor() { // test construction from vector. SYCL_ESIMD_FUNCTION void test_simd_view_from_vector() { - simd s = 0; - simd_view v1 = s; - simd_view v2(s); + simd 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, region_base> v3( + simd_view, region_base> sv16c( (simd())); + + simd v1 = 0; + simd_view sv1a = v1; + simd_view sv1b(v1); } // move constructor transfers the same view of the underlying data. From b4d1d4e8954af7ce1a557facc8b1556e9d751ced Mon Sep 17 00:00:00 2001 From: Vyacheslav N Klochkov Date: Tue, 2 Nov 2021 12:17:14 -0700 Subject: [PATCH 5/5] Address reviewer's comment - convert a comment to doxygen style Signed-off-by: Vyacheslav N Klochkov --- .../include/sycl/ext/intel/experimental/esimd/simd_view.hpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp index 774d6c1e2715c..9000f8fee80f7 100644 --- a/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp +++ b/sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp @@ -68,11 +68,11 @@ class simd_view : public detail::simd_view_impl { /// @} 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 + /// Construct a complete view of a vector simd_view(BaseTy &Base) : BaseClass(Base) {} simd_view &operator=(const simd_view &Other) { @@ -142,7 +142,7 @@ class simd_view> simd_view(BaseTy &&Base, RegionTy Region) : BaseClass(Base, Region) {} public: - // Construct a complete view of a vector + /// Construct a complete view of a vector simd_view(BaseTy &Base) : BaseClass(Base) {} operator element_type() const {