Skip to content

Commit daae147

Browse files
[ESIMD] Allow copy and move constructors for simd_view (#4311)
This patch allows copy and move constructors and now simd_view class adheres to the "rule of 5". We also mark APIs of simd class with [[clang::lifetimebound]] attirbute to warn users when simd_view escapes the lifetime of the underlying simd object.
1 parent 28bfa9a commit daae147

File tree

5 files changed

+112
-15
lines changed

5 files changed

+112
-15
lines changed

sycl/include/sycl/ext/intel/experimental/esimd/detail/simd_view_impl.hpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ template <typename BaseTy, typename RegionTy> class simd_view_impl {
5959
: M_base(Base), M_region(Region) {}
6060

6161
public:
62-
// Disallow copy and move constructors.
63-
simd_view_impl(const simd_view_impl &Other) = delete;
64-
simd_view_impl(simd_view_impl &&Other) = delete;
62+
// Default copy and move constructors.
63+
simd_view_impl(const simd_view_impl &Other) = default;
64+
simd_view_impl(simd_view_impl &&Other) = default;
6565
/// @}
6666

6767
/// Conversion to simd type.
@@ -80,6 +80,11 @@ template <typename BaseTy, typename RegionTy> class simd_view_impl {
8080
simd_view_impl &operator=(const value_type &Val) { return write(Val); }
8181
/// @}
8282

83+
/// Move assignment operator.
84+
simd_view_impl &operator=(simd_view_impl &&Other) {
85+
return write(Other.read());
86+
}
87+
8388
/// @{
8489
/// Region accessors.
8590
static constexpr bool is1D() { return !ShapeTy::Is_2D; }

sycl/include/sycl/ext/intel/experimental/esimd/simd.hpp

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ template <typename Ty, int N> class simd {
139139
}
140140

141141
/// View this simd object in a different element type.
142-
template <typename EltTy> auto bit_cast_view() & {
142+
template <typename EltTy> auto bit_cast_view() &[[clang::lifetimebound]] {
143143
using TopRegionTy = detail::compute_format_type_t<simd, EltTy>;
144144
using RetTy = simd_view<simd, TopRegionTy>;
145145
TopRegionTy R(0);
@@ -153,7 +153,8 @@ template <typename Ty, int N> class simd {
153153
}
154154

155155
/// View as a 2-dimensional simd_view.
156-
template <typename EltTy, int Height, int Width> auto bit_cast_view() & {
156+
template <typename EltTy, int Height, int Width>
157+
auto bit_cast_view() &[[clang::lifetimebound]] {
157158
using TopRegionTy =
158159
detail::compute_format_type_2d_t<simd, EltTy, Height, Width>;
159160
using RetTy = simd_view<simd, TopRegionTy>;
@@ -174,7 +175,8 @@ template <typename Ty, int N> class simd {
174175
/// \param Offset is the starting element offset.
175176
/// \return the representing region object.
176177
template <int Size, int Stride>
177-
simd_view<simd, region1d_t<Ty, Size, Stride>> select(uint16_t Offset = 0) & {
178+
simd_view<simd, region1d_t<Ty, Size, Stride>> select(uint16_t Offset = 0) &[
179+
[clang::lifetimebound]] {
178180
region1d_t<Ty, Size, Stride> Reg(Offset);
179181
return {*this, Reg};
180182
}
@@ -200,7 +202,8 @@ template <typename Ty, int N> class simd {
200202
Ty operator()(int i) const { return data()[i]; }
201203

202204
/// Return writable view of a single element.
203-
simd_view<simd, region1d_t<Ty, 1, 0>> operator[](int i) {
205+
simd_view<simd, region1d_t<Ty, 1, 0>> operator[](int i)
206+
[[clang::lifetimebound]] {
204207
return select<1, 0>(i);
205208
}
206209

sycl/include/sycl/ext/intel/experimental/esimd/simd_view.hpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,9 @@ class simd_view : public detail::simd_view_impl<BaseTy, RegionTy> {
4242
simd_view(BaseTy &&Base, RegionTy Region) : BaseClass(Base, Region) {}
4343

4444
public:
45-
// Disallow copy and move constructors for simd_view.
46-
simd_view(const simd_view &Other) = delete;
47-
simd_view(simd_view &&Other) = delete;
45+
// Default copy and move constructors for simd_view.
46+
simd_view(const simd_view &Other) = default;
47+
simd_view(simd_view &&Other) = default;
4848

4949
/// @{
5050
/// Assignment operators.
@@ -58,6 +58,12 @@ class simd_view : public detail::simd_view_impl<BaseTy, RegionTy> {
5858
}
5959
/// @}
6060

61+
/// Move assignment operator.
62+
simd_view &operator=(simd_view &&Other) {
63+
*this = Other.read();
64+
return *this;
65+
}
66+
6167
#define DEF_RELOP(RELOP) \
6268
ESIMD_INLINE friend simd<uint16_t, length> operator RELOP( \
6369
const simd_view &X, const value_type &Y) { \

sycl/test/esimd/simd_view.cpp

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
using namespace sycl::ext::intel::experimental::esimd;
88

9-
bool test_simd_view_bin_ops() __attribute__((sycl_device)) {
9+
SYCL_ESIMD_FUNCTION bool test_simd_view_bin_ops() {
1010
simd<int, 16> v0 = 1;
1111
simd<int, 16> v1 = 2;
1212
auto ref0 = v0.select<8, 2>(0);
@@ -23,7 +23,7 @@ bool test_simd_view_bin_ops() __attribute__((sycl_device)) {
2323
return v0[0] == 1;
2424
}
2525

26-
bool test_simd_view_unary_ops() __attribute__((sycl_device)) {
26+
SYCL_ESIMD_FUNCTION bool test_simd_view_unary_ops() {
2727
simd<int, 16> v0 = 1;
2828
simd<int, 16> v1 = 2;
2929
auto ref0 = v0.select<8, 2>(0);
@@ -35,21 +35,21 @@ bool test_simd_view_unary_ops() __attribute__((sycl_device)) {
3535
return v1[0] == 1;
3636
}
3737

38-
bool test_simd_view_assign1() __attribute__((sycl_device)) {
38+
SYCL_ESIMD_FUNCTION bool test_simd_view_assign1() {
3939
simd<int, 32> v0(0, 1);
4040
simd<int, 16> v1(0, 1);
4141
v0.select<8, 1>(0) = v1.select<8, 1>(0) + v1.select<8, 1>(1);
4242
return v0[8] == 8 + 9;
4343
}
4444

45-
bool test_simd_view_assign2() __attribute__((sycl_device)) {
45+
SYCL_ESIMD_FUNCTION bool test_simd_view_assign2() {
4646
simd<int, 32> v0 = 0;
4747
simd<int, 16> v1 = 1;
4848
v0.select<8, 1>(0) = v1.select<8, 1>(0);
4949
return v0[0] == 1;
5050
}
5151

52-
bool test_simd_view_assign3() __attribute__((sycl_device)) {
52+
SYCL_ESIMD_FUNCTION bool test_simd_view_assign3() {
5353
simd<int, 64> v0 = 0;
5454
simd<int, 64> v1 = 1;
5555
auto mask = (v0.select<16, 1>(0) > v1.select<16, 1>(0));
@@ -62,6 +62,61 @@ bool test_simd_view_assign3() __attribute__((sycl_device)) {
6262
return val[0] == 0 && val1[0] == 0;
6363
}
6464

65+
// copy constructor creates the same view of the underlying data.
66+
SYCL_ESIMD_FUNCTION void test_simd_view_copy_ctor() {
67+
simd<int, 16> v0 = 1;
68+
auto v0_view = v0.select<8, 1>(0);
69+
auto v0_view_copy(v0_view);
70+
}
71+
72+
// move constructor transfers the same view of the underlying data.
73+
SYCL_ESIMD_FUNCTION void test_simd_view_move_ctor() {
74+
simd<int, 16> v0 = 1;
75+
auto v0_view = v0.select<8, 1>(0);
76+
auto v0_view_move(std::move(v0_view));
77+
}
78+
79+
// assignment operator copies the underlying data.
80+
SYCL_ESIMD_FUNCTION void test_simd_view_copy_assign() {
81+
simd<int, 16> v0 = 0;
82+
simd<int, 16> v1 = 1;
83+
auto v0_view = v0.select<8, 1>(0);
84+
auto v1_view = v1.select<8, 1>(0);
85+
v0_view = v1_view;
86+
}
87+
88+
// move assignment operator copies the underlying data.
89+
SYCL_ESIMD_FUNCTION void test_simd_view_move_assign() {
90+
simd<int, 16> v0 = 0;
91+
simd<int, 16> v1 = 1;
92+
auto v0_view = v0.select<8, 1>(0);
93+
auto v1_view = v1.select<8, 1>(0);
94+
v0_view = std::move(v1_view);
95+
}
96+
97+
// Check that the same holds for specialization with lenght==1.
98+
SYCL_ESIMD_FUNCTION void test_simd_view_ctors_length1() {
99+
simd<int, 16> v = 1;
100+
auto v0_view = v[0];
101+
auto v0_view_copy(v0_view); // copy ctor
102+
auto v0_view_move(std::move(v0_view)); // move ctor
103+
simd<int, 16> vv = 2;
104+
auto vv0_view = vv[0];
105+
v0_view_copy = vv0_view; // copy assign
106+
v0_view_move = std::move(vv0_view); // move assign
107+
}
108+
109+
// Check that simd_view can be passed as a by-value argument
110+
template <class BaseTy, class RegionTy>
111+
SYCL_ESIMD_FUNCTION void foo(simd_view<BaseTy, RegionTy> view) {}
112+
113+
SYCL_ESIMD_FUNCTION void bar() {
114+
simd<int, 16> v0 = 0;
115+
auto v0_view = v0.select<8, 1>(0);
116+
foo(v0_view); // lvalue
117+
foo(v0.select<8, 1>(0)); // rvalue
118+
}
119+
65120
void test_simd_view_subscript() SYCL_ESIMD_FUNCTION {
66121
simd<int, 4> v = 1;
67122
auto vv = v.select<2, 1>(0);
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// RUN: %clangxx -fsycl -fsycl-device-only -fsyntax-only -Xclang -verify %s
2+
3+
#include <sycl/ext/intel/experimental/esimd.hpp>
4+
using namespace sycl::ext::intel::experimental::esimd;
5+
6+
// Explicitly returning simd_view for local objects is wrong,
7+
// and it should be programmers fault, similar to string_view.
8+
// However, sometimes we could return simd_view from a function
9+
// implicitly. This test checks that users will see a warning in such situation.
10+
simd_view<simd<float, 4>, region1d_t<float, 1, 0>> f1(simd<float, 4> x) {
11+
// expected-warning@+1 {{address of stack memory associated with parameter 'x' returned}}
12+
return x[0];
13+
}
14+
15+
simd_view<simd<float, 4>, region1d_t<float, 2, 1>> f2(simd<float, 4> x) {
16+
// expected-warning@+1 {{address of stack memory associated with parameter 'x' returned}}
17+
return x.select<2, 1>(0);
18+
}
19+
20+
auto f3(simd<float, 4> x) {
21+
// expected-warning@+1 {{address of stack memory associated with parameter 'x' returned}}
22+
return x.bit_cast_view<int>();
23+
}
24+
25+
auto f4(simd<float, 4> x) {
26+
// expected-warning@+1 {{address of stack memory associated with parameter 'x' returned}}
27+
return x.bit_cast_view<int, 2, 2>();
28+
}

0 commit comments

Comments
 (0)