[SYCL] Remove IsDeprecatedDeviceCopyable

[aelovikov-intel]

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.

[aelovikov-intel]

A better way might be just making sycl::detail::tuple trivially copyable automatically whenever all the contained types are, by fixing the implementation, but I'm not sure how big such a change would be or how to approach it.

[aelovikov-intel]

Ping

[aelovikov-intel]

Looks like this broke CTS. Root cause: https://godbolt.org/z/vqan5Y9xM, so the issue is either in the CTS or in the clang FE.

--- a/tests/handler/handler_invoke_api.h
+++ b/tests/handler/handler_invoke_api.h
@@ -43,6 +43,7 @@ struct TestConstants {
         offsetNdRange(offsetRange, offsetRange, offset),
         numWorkGroups(1),
         workGroupSize(defaultRange) {}
+  ~TestConstants() = default;
 };

 struct use_offset {

in CTS would make it buildable again.

@intel/dpcpp-cfe-reviewers , can you tell if clang's behavior is correct?

[elizabethandrews]

Looks like this broke CTS. Root cause: https://godbolt.org/z/vqan5Y9xM, so the issue is either in the CTS or in the clang FE.

--- a/tests/handler/handler_invoke_api.h
+++ b/tests/handler/handler_invoke_api.h
@@ -43,6 +43,7 @@ struct TestConstants {
         offsetNdRange(offsetRange, offsetRange, offset),
         numWorkGroups(1),
         workGroupSize(defaultRange) {}
+  ~TestConstants() = default;
 };

 struct use_offset {

in CTS would make it buildable again.

@intel/dpcpp-cfe-reviewers , can you tell if clang's behavior is correct?

Hmm... It is unclear why the defaulted destructor changes the behavior in clang. I also noticed that removing the const member changes the behavior. There seems to be some underlying bug but it is unclear to me whether TestConstants is trivially copyable or not. I am looking at the standards now. Tom is also looking into this.

[elizabethandrews]

Looks like this broke CTS. Root cause: https://godbolt.org/z/vqan5Y9xM, so the issue is either in the CTS or in the clang FE.

--- a/tests/handler/handler_invoke_api.h
+++ b/tests/handler/handler_invoke_api.h
@@ -43,6 +43,7 @@ struct TestConstants {
         offsetNdRange(offsetRange, offsetRange, offset),
         numWorkGroups(1),
         workGroupSize(defaultRange) {}
+  ~TestConstants() = default;
 };

 struct use_offset {

in CTS would make it buildable again.
@intel/dpcpp-cfe-reviewers , can you tell if clang's behavior is correct?

Hmm... It is unclear why the defaulted destructor changes the behavior in clang. I also noticed that removing the const member changes the behavior. There seems to be some underlying bug but it is unclear to me whether TestConstants is trivially copyable or not. I am looking at the standards now. Tom is also looking into this.

I discussed this with Tom and we think this is a Clang bug. Can you file a JIRA for it?

[tahonermann]

std::is_trivially_copyable is an odd trait since it can be satisfied in so many ways. For example, a type that is trivially move-assignable but not trivially copy/move constructible or trivially copy-assignable satisfies it. I suggest avoiding it and using the more precise std::is_trivially_copy_constructible traits to specify the actual requirements needed.

[jsji]

I discussed this with Tom and we think this is a Clang bug. Can you file a JIRA for it?

64941, assigned to you first.