Clarify that certain types are trivially copyable

[Pennycook]

id, range, nd_range, vec and marray are permitted as kernel arguments, but the specification does not say that they are trivially copyable nor that they are device copyable.

Closes internal issue 564.

[TApplencourt]

id, range, nd_range, vec and marray are permitted as kernel arguments

I did a quick ctrl+f and found nothing. Can you share the section where we said that?

I found #210,
We said they follow "4.5.3. Common by-value semantics", https://registry.khronos.org/SYCL/specs/sycl-2020/html/sycl-2020.html#sec:byval-semantics

So should "4.5.3. Common by-value semantics" include "trivially copyable"?

[Pennycook]

I did a quick ctrl+f and found nothing. Can you share the section where we said that?

I don't know where it exists; @gmlueck opened the internal issue quite a while ago, and whatever text he was referencing might have been changed. But several people weighed in at the time with the opinion that this was the intent.

So should "4.5.3. Common by-value semantics" include "trivially copyable"?

No, I don't think so. group, sub_group and nd_item are not trivially copyable. item could be trivially copyable if we defined it to be nothing more than a container for an id and a range, but that's still being discussed.

[TApplencourt]

range, id, etc... are host-constructible, so indeed one can try to pass them as kernel argument.

I'm always afraid to duplicate so much the are trivially copyable, it make refactoring harder. So I was thinking if we can add it to just one place, it will simplify the spec.

But yeah, group are not trivially copyable (but they are not host-constructible, so nobody can ever know :p) so cannot do that.

[Pennycook]

I'm always afraid to duplicate so much the are trivially copyable, it make refactoring harder. So I was thinking if we can add it to just one place, it will simplify the spec.

I agree with this. The view I tend to take, though, is that it's most important that the specification says the right thing before we worry about where we say it. So, in this case, I'd like to get the correct text in and the CTS tests done, and then we can always refactor things later.

[tomdeakin]

Awaiting CTS

[keryell]

Thanks!

[Pennycook]

Just to follow-up on today's discussion about marray... The definition of "numeric type" in ISO C++ (https://eel.is/c++draft/numeric.requirements#1) is:

The complex and valarray components are parameterized by the type of information they contain and manipulate. A C++ program shall instantiate these components only with a numeric type. A numeric type is a cv-unqualified object type T that meets the Cpp17DefaultConstructible, Cpp17CopyConstructible, Cpp17CopyAssignable, and Cpp17Destructible requirements ([utility.arg.requirements]).242

I think this means that you can have a "numeric type" that isn't a "trivially copyable" type. So you can create an marray from something that isn't trivially copyable.

[gmlueck]

I think this change is good, but we can now clean up the specification in "Rules for parameter passing to kernels". These bullet items can now be removed:

• id;
• range;
• marray<T, NumElements> when T is device copyable;
• vec<T, NumElements>.

This is because this PR guarantees that these types are trivially copyable, and we already say that any trivially copyable type is a valid kernel parameter.

[Pennycook]

These bullet items can now be removed:

• id;
• range;
• marray<T, NumElements> when T is device copyable;
• vec<T, NumElements>.

I always like it when we make the specification simpler. 😄 I've applied this suggestion in ae34962.

[gmlueck]

Please resolve the merge conflicts.

[Pennycook]

Please resolve the merge conflicts.

I think it was just the vector_t stuff. I'd appreciate it if you could double-check that the merge commit looks right.

[gmlueck]

Please resolve the merge conflicts.

I think it was just the vector_t stuff. I'd appreciate it if you could double-check that the merge commit looks right.

Looks good, thanks!

[tomdeakin]

Still waiting CTS.

[gmlueck]

For my own benefit, the Intel tracker for the CTS is CMPLRTST-26896