std::is_trivially_copyable too strong as a constraint

[krzikalla]

The line

alpaka/include/alpaka/kernel/Traits.hpp

Line 227 in 4c981f0

std::is_empty_v<T> || std::is_trivially_copyable_v<T>,

constrains types to copyable ones by just using std::is_trivially_copyable. Our users sometimes have types, which are trivially copyable, but don't fulfill the requirements of std::is_trivially_copyable. It is not allowed to specialize std::is_trivially_copyable itself.
Is it possible to get a trait for this property, so that a user is able to specify trivial copyability for particular types?

[fwyzard]

hi @krzikalla

Our users sometimes have types, which are trivially copyable, but don't fulfill the requirements of std::is_trivially_copyable.

could you provide an example of such a type ?

[krzikalla]

include/codi/expressions/activeType.hpp

is an example. For some template types T_Tape ActiveType is trivial. Still, it has a destructor and hence std::is_trivially_copyable is false.

[fwyzard]

OK - for this specific case, could you change the destructor from

CODI_INLINE ~ActiveType() {}

to

CODI_INLINE ~ActiveType() = default;

?

This should make it a trivially copyable type :-)

---

In general, one the concerns with non-trivially copyable types related to destructors is - if the type has a non-trivial destructor, should it be called inside the kernel, when the objects go out of scope ?

[janbac]

Even though the template ActiveType looks complicated, it sometimes results in a trivially copyable type, e.g.
codi::RealForward
https://github.com/SciCompKL/CoDiPack/blob/89c4ffe314f0382d9b9c4aa760c6dd1627d74d7c/include/codi.hpp#L95C3-L95C3

Its state only consist of two doubles, which can be copied using memcpy.
However, is_trivially_copyable can not check this, as there are e.g. custom destructors (which do nothing in case of RealForward)

[bernhardmgruber]

I had a quick look at the CoDiPack project and defaulting the destructor does not make the type trivially copyable, because it is still not trivially destructible. The reason is that ~ActiveTypeBase() calls Base::destroy(), which calls LhsExpressionInterface::destroy() IIUC, which calls Impl::getTape().destroyIdentifier(cast().value(), cast().getIdentifier());, etc. So destroying a value of e.g. RealForward is indeed a complex operation.

I think alpaka is right in rejecting to byte-wise copy such a type to an accelerator, since it would break the guarantees of the type system.

Now I also understand your desire to hack the type system, in which case you could e.g. bluntly cast away the type before passing a value to a kernel, e.g. casting it to void* or a std::byte[sizeof(T)]. Can you please try if this works for you?

[psychocoderHPC]

@krzikalla IMO if you are 100% sure that the type is allowed to be copied via memcopy you could in principle specialize std::is_trivially_copyable for your type. This should be better than casting the type to void*.

[krzikalla]

@psychocoderHPC : forbidden by the standard.

[psychocoderHPC]

@psychocoderHPC : forbidden by the standard.

Ohh missed that in the standard. :-(
In that case the suggestion @bernhardmgruber made with writing a wrapper that is transparently storing the data as std::byte[sizeof(T)] and is dereferencing it on the device side sounds the best solution.

[fwyzard]

@krzikalla

@psychocoderHPC : forbidden by the standard.

According to CppReference (I don't have the standard at hand) it's not "forbidden", it's "undefined behaviour":

The behavior of a program that adds specializations for std::is_trivially_copyable or std::is_trivially_copyable_v is undefined.

---

@psychocoderHPC, @bernhardmgruber:

In that case the suggestion @bernhardmgruber made with writing a wrapper that is transparently storing the data as std::byte[sizeof(T)] and is dereferencing it on the device side sounds the best solution.

Technically true, but it's definitely cumbersome as a user interface.

The request to have something like

namespace alpaka {
  template<typename T>
  struct is_trivially_copyable : public std::false_type {};
}

that users can override specialise as

namespace alpaka {
  template<>
  struct is_trivially_copyable<MyType> : public std::true_type {};
}

seems reasonable enough...

[bernhardmgruber]

According to CppReference (I don't have the standard at hand) it's not "forbidden", it's "undefined behaviour":

Which is arguably worse, UB means "behavior for which this document imposes no requirements", see current draft. So the program can literally do anything.

I want to add that my suggestion of erasing the type to bypass the test is equally UB.

The request to have something like

namespace alpaka {
  template<typename T>
  struct is_trivially_copyable : public std::false_type {};
}

[...]
seems reasonable enough...

I may be on board if we name it differently so it communicates intent and consequences. Something like:

namespace alpaka {
  template <typename T>
  inline constexpr bool allowMemcpyToDeviceAndIWillLiveWithTheConsequences = ...;
}

Anyway, this is not only my decision.

[fwyzard]

Which is arguably worse, UB means "behavior for which this document imposes no requirements", see current draft. So the program can literally do anything.

... including the actually intended behaviour :-)

I want to add that my suggestion of erasing the type to bypass the test is equally UB.

Agreed.

[psychocoderHPC]

What is if we change our check and use our own trait to check for the required condition which can be specialized by the user?
We already check for is_empty() and trivially_copyable(). If I remember correctly using is_empty() was already a workaround from our side for lambda functions.

So I suggest alpaka::isMemCopyable<> and the default if not specified for a type is std::is_empty_v<T> || std::is_trivially_copyable_v<T>.

@bernhardmgruber @fwyzard

[bernhardmgruber]

We already check for is_empty() and trivially_copyable(). If I remember correctly using is_empty() was already a workaround from our side for lambda functions.

is_empty() was necessary to support lambdas passed to kernels, because it is implementation defined whether they are trivially copyable or not.

So I suggest alpaka::isMemCopyable<> and the default if not specified for a type is std::is_empty_v || std::is_trivially_copyable_v.

Fine. Please go ahead and provide a PR :)

[MaxSagebaum]

Just some additional background information:

@janbac discussed this topic with me yesterday and I did some tests.

It would be possible to implement a CoDiPack type that is trivially copyable. Here we would need to define default versions for the constructor, destructor and the copy operator (operator=) in the classes ActiveType, ActiveTypeBase and LhsExpressionInterface.

• These alternative versions could be implemented with preprocessor macros. Since it is not true for all versions of ActiveType this would be wrong.
• SFINAE is not possible since the methods do not have template arguments (and can not have them), therefore the compiler creates errors for the SFINAE expressions were the argument is false.
• It could be implemented with C++20 with restrict but we are currently limiting us to Cpp11. We might switch to Cpp14 which would also be to low.
• A direct implementation would need to copy ActiveType, ActiveTypeBase and LhsExpressionInterface and change basically 3 lines. Since this are copy operations, it is problematic to use inheritance, to remove the duplicated code. Mixins would be better for this kind of implementation.

According to CppReference (I don't have the standard at hand) it's not "forbidden", it's "undefined behaviour":

My personal opinion on that, is that the formulation is wrong. I think the authors wanted to say: If a type is not trivially copyable and a specialization of std::is_trivially_copyable changes this. then the behavior is undefined.

[bernhardmgruber]

• These alternative versions could be implemented with preprocessor macros. Since it is not true for all versions of ActiveType this would be wrong.

Special casing through the preprocessor is also bad. But I also get that it may be difficult to solve in pure C++. @psychocoderHPC is preparing a PR to supply you the escape hatch you need.

• A direct implementation would need to copy ActiveType, ActiveTypeBase and LhsExpressionInterface and change basically 3 lines. Since this are copy operations, it is problematic to use inheritance, to remove the duplicated code. Mixins would be better for this kind of implementation.

Inheritance should work perfectly fine with trivial/defaulted implementations for construction, copy and move operations. And mixins in C++ are just a different form of inheritance, often coupled with CRTP. But as I said, if it's too difficult, you can use the escape hatch.

According to CppReference (I don't have the standard at hand) it's not "forbidden", it's "undefined behaviour":

My personal opinion on that, is that the formulation is wrong. I think the authors wanted to say: If a type is not trivially copyable and a specialization of std::is_trivially_copyable changes this. then the behavior is undefined.

Well, the wording of the C++26 working draft says that: "Unless otherwise specified, the behavior of a program that adds specializations for any of the templates specified in [type.traits] is undefined. See here. And std::is_trivially_copyable is specified in this aforementioned section.

[MaxSagebaum]

Thanks for the link to the standard draft. That clears things up. Do you know of a document that explains the reasoning behind this choice?

[bernhardmgruber]

Thanks for the link to the standard draft. That clears things up. Do you know of a document that explains the reasoning behind this choice?

I don't, but you may be lucky posting to the std-discussion mailing list, or asking on reddit.

[psychocoderHPC]

I added the trait that gives the user the possibility to relax the kernel argument conditions at their own risk in #2198

[jasminmohnke]

While this change clears the issue with the check on kernel arguments being trivially copyable, unfortunately I am running into a comparable issue with the check done on the kernel itself.
If, for instance, a kernel has a member that is of a type such as described previously, std::is_trivially_copyable<Kernel> evaluates to false as well. A trait similarly to the one you introduced for the kernel argument would be helpful to have for the kernel as well.