Skip to content

Commit a5e8209

Browse files
[SYCL] Remove IsDeprecatedDeviceCopyable (#16615)
This was discussed in #15342 (comment) and the consensus seemed to be that we should drop it right away in a separate PR, do it here. Technically, it is a breaking change that could also be considered a bugfix. An example of a class failing the updated check is ``` struct Kernel { Kernel(int); Kernel(const Kernel&) = default; Kernel& operator=(const Kernel&) { return *this; } // non-trivial }; ``` An additional minor reason (other than not being SYCL-conformant) to drop it right away is to save a tiny bit of compile time that is currently used to support something violating the spec. This required some fixes in the reductions implementation to make sure the kernel we submit internally are actually device copyable.
1 parent 98c0d5d commit a5e8209

File tree

3 files changed

+13
-31
lines changed

3 files changed

+13
-31
lines changed

sycl/include/sycl/detail/is_device_copyable.hpp

+8-20
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ inline namespace _V1 {
3131
template <typename T> struct is_device_copyable;
3232

3333
namespace detail {
34+
template <typename... T> struct tuple;
35+
3436
template <typename T, typename = void>
3537
struct is_device_copyable_impl : std::is_trivially_copyable<T> {};
3638

@@ -70,6 +72,10 @@ template <typename... Ts>
7072
struct is_device_copyable<std::tuple<Ts...>>
7173
: std::bool_constant<(... && is_device_copyable<Ts>::value)> {};
7274

75+
template <typename... Ts>
76+
struct is_device_copyable<sycl::detail::tuple<Ts...>>
77+
: std::bool_constant<(... && is_device_copyable<Ts>::value)> {};
78+
7379
// std::variant<Ts...> is implicitly device copyable type if each type T of
7480
// Ts... is device copyable.
7581
template <typename... Ts>
@@ -83,31 +89,14 @@ struct is_device_copyable<T[N]> : is_device_copyable<T> {};
8389
template <typename T>
8490
inline constexpr bool is_device_copyable_v = is_device_copyable<T>::value;
8591
namespace detail {
86-
template <typename T, typename = void>
87-
struct IsDeprecatedDeviceCopyable : std::false_type {};
88-
89-
// TODO: using C++ attribute [[deprecated]] or the macro __SYCL2020_DEPRECATED
90-
// does not produce expected warning message for the type 'T'.
91-
template <typename T>
92-
struct __SYCL2020_DEPRECATED("This type isn't device copyable in SYCL 2020")
93-
IsDeprecatedDeviceCopyable<
94-
T, std::enable_if_t<std::is_trivially_copy_constructible_v<T> &&
95-
std::is_trivially_destructible_v<T> &&
96-
!is_device_copyable_v<T>>> : std::true_type {};
97-
98-
template <typename T, int N>
99-
struct __SYCL2020_DEPRECATED("This type isn't device copyable in SYCL 2020")
100-
IsDeprecatedDeviceCopyable<T[N]> : IsDeprecatedDeviceCopyable<T> {};
101-
10292
#ifdef __SYCL_DEVICE_ONLY__
10393
// Checks that the fields of the type T with indices 0 to (NumFieldsToCheck -
10494
// 1) are device copyable.
10595
template <typename T, unsigned NumFieldsToCheck>
10696
struct CheckFieldsAreDeviceCopyable
10797
: CheckFieldsAreDeviceCopyable<T, NumFieldsToCheck - 1> {
10898
using FieldT = decltype(__builtin_field_type(T, NumFieldsToCheck - 1));
109-
static_assert(is_device_copyable_v<FieldT> ||
110-
detail::IsDeprecatedDeviceCopyable<FieldT>::value,
99+
static_assert(is_device_copyable_v<FieldT>,
111100
"The specified type is not device copyable");
112101
};
113102

@@ -119,8 +108,7 @@ template <typename T, unsigned NumBasesToCheck>
119108
struct CheckBasesAreDeviceCopyable
120109
: CheckBasesAreDeviceCopyable<T, NumBasesToCheck - 1> {
121110
using BaseT = decltype(__builtin_base_type(T, NumBasesToCheck - 1));
122-
static_assert(is_device_copyable_v<BaseT> ||
123-
detail::IsDeprecatedDeviceCopyable<BaseT>::value,
111+
static_assert(is_device_copyable_v<BaseT>,
124112
"The specified type is not device copyable");
125113
};
126114

sycl/include/sycl/reduction.hpp

+5-1
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ class ReductionIdentityContainer<
389389
static constexpr bool has_identity = true;
390390

391391
ReductionIdentityContainer(const T &) {}
392-
ReductionIdentityContainer() {}
392+
ReductionIdentityContainer() = default;
393393

394394
/// Returns the statically known identity value.
395395
static constexpr T getIdentity() {
@@ -407,6 +407,10 @@ class ReductionIdentityContainer<
407407

408408
ReductionIdentityContainer(const T &Identity) : MIdentity(Identity) {}
409409

410+
// Make it trivially copyable (need at least on of the special member
411+
// functions):
412+
ReductionIdentityContainer(const ReductionIdentityContainer &) = default;
413+
410414
/// Returns the identity value given by user.
411415
T getIdentity() const { return MIdentity; }
412416

sycl/test/basic_tests/is_device_copyable.cpp

-10
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,6 @@ struct BCopyable {
2525
BCopyable(const BCopyable &x) : i(x.i) {}
2626
};
2727

28-
// Not trivially copyable, but trivially copy constructible/destructible.
29-
// Such types are passed to kernels to stay compatible with deprecated
30-
// sycl 1.2.1 rules.
31-
struct C : A {
32-
const A C2;
33-
C() : A{0}, C2{2} {}
34-
};
35-
3628
// Not copyable type, but it will be declared as device copyable.
3729
struct DCopyable {
3830
int i;
@@ -67,7 +59,6 @@ void test() {
6759
A IamGood;
6860
IamGood.i = 0;
6961
BCopyable IamBadButCopyable(1);
70-
C IamAlsoGood;
7162
DCopyable IamAlsoBadButCopyable{0};
7263
marray<int, 5> MarrayForCopyableIsCopyable(0);
7364
range<2> Range{1,2};
@@ -78,7 +69,6 @@ void test() {
7869
int A = IamGood.i;
7970
int B = IamBadButCopyable.i;
8071
int C = IamAlsoBadButCopyable.i;
81-
int D = IamAlsoGood.i;
8272
int E = MarrayForCopyableIsCopyable[0];
8373
int F = Range[1];
8474
int G = Id[2];

0 commit comments

Comments
 (0)