From bec21d1c457cdac410ae71ee9dc1caff9db3d1c3 Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Mon, 13 Jan 2025 15:56:31 -0800 Subject: [PATCH 1/2] [SYCL] Remove `IsDeprecatedDeviceCopyable` This was discussed in https://github.com/intel/llvm/pull/15342#discussion_r1751868640 and the consensus seemd 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. --- .../sycl/detail/is_device_copyable.hpp | 28 ++++++------------- sycl/include/sycl/reduction.hpp | 10 ++++++- sycl/test/basic_tests/is_device_copyable.cpp | 10 ------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/sycl/include/sycl/detail/is_device_copyable.hpp b/sycl/include/sycl/detail/is_device_copyable.hpp index 388029e6a16a3..a5dcd9d0235f1 100644 --- a/sycl/include/sycl/detail/is_device_copyable.hpp +++ b/sycl/include/sycl/detail/is_device_copyable.hpp @@ -31,6 +31,8 @@ inline namespace _V1 { template struct is_device_copyable; namespace detail { +template struct tuple; + template struct is_device_copyable_impl : std::is_trivially_copyable {}; @@ -70,6 +72,10 @@ template struct is_device_copyable> : std::bool_constant<(... && is_device_copyable::value)> {}; +template +struct is_device_copyable> + : std::bool_constant<(... && is_device_copyable::value)> {}; + // std::variant is implicitly device copyable type if each type T of // Ts... is device copyable. template @@ -83,22 +89,6 @@ struct is_device_copyable : is_device_copyable {}; template inline constexpr bool is_device_copyable_v = is_device_copyable::value; namespace detail { -template -struct IsDeprecatedDeviceCopyable : std::false_type {}; - -// TODO: using C++ attribute [[deprecated]] or the macro __SYCL2020_DEPRECATED -// does not produce expected warning message for the type 'T'. -template -struct __SYCL2020_DEPRECATED("This type isn't device copyable in SYCL 2020") - IsDeprecatedDeviceCopyable< - T, std::enable_if_t && - std::is_trivially_destructible_v && - !is_device_copyable_v>> : std::true_type {}; - -template -struct __SYCL2020_DEPRECATED("This type isn't device copyable in SYCL 2020") - IsDeprecatedDeviceCopyable : IsDeprecatedDeviceCopyable {}; - #ifdef __SYCL_DEVICE_ONLY__ // Checks that the fields of the type T with indices 0 to (NumFieldsToCheck - // 1) are device copyable. @@ -106,8 +96,7 @@ template struct CheckFieldsAreDeviceCopyable : CheckFieldsAreDeviceCopyable { using FieldT = decltype(__builtin_field_type(T, NumFieldsToCheck - 1)); - static_assert(is_device_copyable_v || - detail::IsDeprecatedDeviceCopyable::value, + static_assert(is_device_copyable_v, "The specified type is not device copyable"); }; @@ -119,8 +108,7 @@ template struct CheckBasesAreDeviceCopyable : CheckBasesAreDeviceCopyable { using BaseT = decltype(__builtin_base_type(T, NumBasesToCheck - 1)); - static_assert(is_device_copyable_v || - detail::IsDeprecatedDeviceCopyable::value, + static_assert(is_device_copyable_v, "The specified type is not device copyable"); }; diff --git a/sycl/include/sycl/reduction.hpp b/sycl/include/sycl/reduction.hpp index 576dac72a3ce2..a3c09771662de 100644 --- a/sycl/include/sycl/reduction.hpp +++ b/sycl/include/sycl/reduction.hpp @@ -389,7 +389,7 @@ class ReductionIdentityContainer< static constexpr bool has_identity = true; ReductionIdentityContainer(const T &) {} - ReductionIdentityContainer() {} + ReductionIdentityContainer() = default; /// Returns the statically known identity value. static constexpr T getIdentity() { @@ -407,6 +407,10 @@ class ReductionIdentityContainer< ReductionIdentityContainer(const T &Identity) : MIdentity(Identity) {} + // Make it trivially copyable (need at least on of the special member + // functions): + ReductionIdentityContainer(const ReductionIdentityContainer &) = default; + /// Returns the identity value given by user. T getIdentity() const { return MIdentity; } @@ -2334,6 +2338,10 @@ void reduCGFuncMulti(handler &CGH, KernelType KernelFunc, reduction::strategy::multi, decltype(KernelTag)>; + static_assert(is_device_copyable_v); + static_assert(is_device_copyable_v); + static_assert(is_device_copyable_v); + CGH.parallel_for(Range, Properties, [=](nd_item NDIt) { // We can deduce IsOneWG from the tag type. constexpr bool IsOneWG = diff --git a/sycl/test/basic_tests/is_device_copyable.cpp b/sycl/test/basic_tests/is_device_copyable.cpp index 1c4199e954530..3e48bd5d77857 100644 --- a/sycl/test/basic_tests/is_device_copyable.cpp +++ b/sycl/test/basic_tests/is_device_copyable.cpp @@ -25,14 +25,6 @@ struct BCopyable { BCopyable(const BCopyable &x) : i(x.i) {} }; -// Not trivially copyable, but trivially copy constructible/destructible. -// Such types are passed to kernels to stay compatible with deprecated -// sycl 1.2.1 rules. -struct C : A { - const A C2; - C() : A{0}, C2{2} {} -}; - // Not copyable type, but it will be declared as device copyable. struct DCopyable { int i; @@ -67,7 +59,6 @@ void test() { A IamGood; IamGood.i = 0; BCopyable IamBadButCopyable(1); - C IamAlsoGood; DCopyable IamAlsoBadButCopyable{0}; marray MarrayForCopyableIsCopyable(0); range<2> Range{1,2}; @@ -78,7 +69,6 @@ void test() { int A = IamGood.i; int B = IamBadButCopyable.i; int C = IamAlsoBadButCopyable.i; - int D = IamAlsoGood.i; int E = MarrayForCopyableIsCopyable[0]; int F = Range[1]; int G = Id[2]; From 109d32019985994f1a79a3c284e5d9b4c9c09c6f Mon Sep 17 00:00:00 2001 From: Andrei Elovikov Date: Fri, 17 Jan 2025 08:25:37 -0800 Subject: [PATCH 2/2] Remove debugging asserts --- sycl/include/sycl/reduction.hpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/sycl/include/sycl/reduction.hpp b/sycl/include/sycl/reduction.hpp index a3c09771662de..7f006648b6cad 100644 --- a/sycl/include/sycl/reduction.hpp +++ b/sycl/include/sycl/reduction.hpp @@ -2338,10 +2338,6 @@ void reduCGFuncMulti(handler &CGH, KernelType KernelFunc, reduction::strategy::multi, decltype(KernelTag)>; - static_assert(is_device_copyable_v); - static_assert(is_device_copyable_v); - static_assert(is_device_copyable_v); - CGH.parallel_for(Range, Properties, [=](nd_item NDIt) { // We can deduce IsOneWG from the tag type. constexpr bool IsOneWG =