Skip to content

Commit 26b058f

Browse files
authored
[SYCL][ESIMD] Add test proxy for move constructors (#5132)
Because copy constructor works as the default fallback for every case with move constructor disabled or not provided, there is no way for a caller to differentiate between actual call to move constructor and copy constructor. Same goes for copy and move assignment operators. Therefore a test proxy is the only way left to check the move constructor and the move assignment works as expected. The drawback of this patch is a requirement to have `__esimd_move_test_proxy` explicitly stated for every user-defined move constructor and move operator. We might use default value for any constructor, the flag will be triggered as needed by any defaulted/implicit move constructor/operator, but user-defined move constructors and operators still need some explicit trigger provided by developer. Signed-off-by: Yuriy Kochetkov <[email protected]>
1 parent 1c9f9f8 commit 26b058f

File tree

3 files changed

+125
-1
lines changed

3 files changed

+125
-1
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <sycl/ext/intel/experimental/esimd/detail/intrin.hpp>
1515
#include <sycl/ext/intel/experimental/esimd/detail/memory_intrin.hpp>
1616
#include <sycl/ext/intel/experimental/esimd/detail/sycl_util.hpp>
17+
#include <sycl/ext/intel/experimental/esimd/detail/test_proxy.hpp>
1718
#include <sycl/ext/intel/experimental/esimd/detail/type_format.hpp>
1819
#include <sycl/ext/intel/experimental/esimd/simd_view.hpp>
1920

@@ -729,11 +730,17 @@ class simd_obj_impl {
729730
__ESIMD_DEF_SIMD_OBJ_IMPL_OPASSIGN(/, /=, __ESIMD_ARITH_OP_FILTER)
730731
#undef __ESIMD_ARITH_OP_FILTER
731732

733+
// Getter for the test proxy member, if enabled
734+
__ESIMD_DECLARE_TEST_PROXY_ACCESS
735+
732736
private:
733737
// The underlying data for this vector.
734738
raw_vector_type M_data;
735739

736740
protected:
741+
// The test proxy if enabled
742+
__ESIMD_DECLARE_TEST_PROXY
743+
737744
void set(const raw_vector_type &Val) {
738745
#ifndef __SYCL_DEVICE_ONLY__
739746
M_data = Val;

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#pragma once
1212

1313
#include <sycl/ext/intel/experimental/esimd/detail/intrin.hpp>
14+
#include <sycl/ext/intel/experimental/esimd/detail/test_proxy.hpp>
1415
#include <sycl/ext/intel/experimental/esimd/detail/type_format.hpp>
1516

1617
__SYCL_INLINE_NAMESPACE(cl) {
@@ -305,8 +306,12 @@ class simd_view_impl {
305306
Derived &operator=(const value_type &Val) { return write(Val); }
306307

307308
/// Move assignment operator.
308-
Derived &operator=(Derived &&Other) { return write(Other.read()); }
309+
Derived &operator=(Derived &&Other) {
310+
__esimd_move_test_proxy(Other);
311+
return write(Other.read());
312+
}
309313
simd_view_impl &operator=(simd_view_impl &&Other) {
314+
__esimd_move_test_proxy(Other);
310315
return write(Other.read());
311316
}
312317

@@ -495,10 +500,17 @@ class simd_view_impl {
495500
return read().all();
496501
}
497502

503+
public:
504+
// Getter for the test proxy member, if enabled
505+
__ESIMD_DECLARE_TEST_PROXY_ACCESS
506+
498507
protected:
499508
// The reference to the base object, which must be a simd object
500509
BaseTy &M_base;
501510

511+
// The test proxy if enabled
512+
__ESIMD_DECLARE_TEST_PROXY
513+
502514
// The region applied on the base object. Its type could be
503515
// - region1d_t
504516
// - region2d_t
Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
//==-------------- test_proxy.hpp - DPC++ Explicit SIMD API ----------------==//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
// Test proxy to differentiate move and copy constructor calls
9+
//===----------------------------------------------------------------------===//
10+
11+
#pragma once
12+
13+
// The test proxy if solely for the test purposes, so it's off by default, with
14+
// no any code generated. It is enabled only if the __ESIMD_ENABLE_TEST_PROXY
15+
// macro is defined.
16+
// It's expected for the proxy class to be available in device code, so that it
17+
// could be incorporated into the ESIMD API classes. Though there is no reason
18+
// to limit it to the __SYCL_DEVICE_ONLY__.
19+
#ifndef __ESIMD_ENABLE_TEST_PROXY
20+
21+
// No code generation by default
22+
#define __ESIMD_DECLARE_TEST_PROXY
23+
#define __ESIMD_DECLARE_TEST_PROXY_ACCESS
24+
#define __esimd_move_test_proxy(other)
25+
26+
#else
27+
28+
// Declare the class attribute
29+
//
30+
// We are using non static data member initialization approach to force
31+
// the value required. Initialization will take place even if no
32+
// default/copy/move constructor of the test_proxy class was explcitly
33+
// called by any of the user-defined constructors of the proxy target
34+
#define __ESIMD_DECLARE_TEST_PROXY \
35+
esimd::detail::test::test_proxy M_testProxy = \
36+
esimd::detail::test::test_proxy();
37+
38+
// Declare the getter to access the proxy from the tests
39+
#define __ESIMD_DECLARE_TEST_PROXY_ACCESS \
40+
const auto &get_test_proxy() const { return M_testProxy; }
41+
42+
// Test proxy will be handled in a proper way by default/implicit move
43+
// constructors and move operators.
44+
// Still the user-defined constructors or move operators should explicitly state
45+
// what to do with each of class atributes, so a proper wrapper required
46+
//
47+
// We are using a simple do-while trick to make sure no code breakage could
48+
// possibly occur in case macro becomes multistatement (PRE10-C in SEI CERT C)
49+
#define __esimd_move_test_proxy(other) \
50+
do { \
51+
M_testProxy = std::move(other.M_testProxy); \
52+
} while (false)
53+
54+
__SYCL_INLINE_NAMESPACE(cl) {
55+
namespace sycl::ext::intel::experimental::esimd::detail::test {
56+
57+
// The test_proxy class.
58+
// Being intended solely for the test purposes, it is enabled only if the
59+
// __ESIMD_ENABLE_TEST_PROXY macro is defined, which is off by default.
60+
//
61+
// This is a helper class for tests to differentiate between the copy
62+
// constructor/assignment and the move constructor/assignment calls,
63+
// as the copy constructor works as the default fallback for every case with
64+
// move constructor disabled or not provided
65+
//
66+
// It is expected for the class with the test proxy (the class under test) to:
67+
// - provide the get_test_proxy() method
68+
// - properly handle moving the test_proxy member in user-defined move
69+
// constructors and user-defined assignment operators
70+
//
71+
// Therefore the following expression is expected to return `true` only if the
72+
// move constructor or move operator was called for the instance of the class
73+
// under test:
74+
// instance.get_test_proxy().was_move_destination()
75+
//
76+
class test_proxy {
77+
// Define the default value to use for every constructor
78+
bool M_move_destination = false;
79+
80+
public:
81+
test_proxy() { __esimd_dbg_print(test_proxy()); }
82+
83+
test_proxy(const test_proxy &) {
84+
__esimd_dbg_print(test_proxy(const test_proxy &other));
85+
}
86+
test_proxy(test_proxy &&) {
87+
__esimd_dbg_print(test_proxy(test_proxy && other));
88+
M_move_destination = true;
89+
}
90+
test_proxy &operator=(const test_proxy &) {
91+
__esimd_dbg_print(test_proxy::operator=(const test_proxy &other));
92+
return *this;
93+
}
94+
test_proxy &operator=(test_proxy &&) {
95+
__esimd_dbg_print(test_proxy::operator=(test_proxy &&other));
96+
M_move_destination = true;
97+
return *this;
98+
}
99+
bool was_move_destination() const { return M_move_destination; }
100+
};
101+
102+
} // namespace sycl::ext::intel::experimental::esimd::detail::test
103+
} // __SYCL_INLINE_NAMESPACE(cl)
104+
105+
#endif // __ESIMD_ENABLE_TEST_PROXY

0 commit comments

Comments
 (0)