Ability to bypass the kernel argument validity check + Eigen compatibility

[ralwing]

Hi,
I'm getting back with some library incompatibility issue.
In my branch https://github.com/ralwing/cuda-api-wrappers/tree/eigen-compat, commit and ralwing@aed66ce
which has some custom changes (e.g by default it compiles as c++17) i test Eigen::Vector and cuda::std::span
The main problem I face here is the requirement:

 static assertion failed with "All kernel parameter types must be of a trivially copy-constructible (decayed) type."

which are already mentioned #551 and #642.

From my understanding of the argument passing documentation Eigen fullfils the requirement to be passed as a kernel argument, although it doesn't meet the std::is_trivially_copyconstructible trait.

So the main problem with this assertion is that it doesn't allow usage of the Eigen structures, as kernel vector. In my code i commented out the part that doesn't compile:

	   auto launch_config =
	       cuda::launch_config_builder().overall_size(1).block_size(256).build();
 
	   auto A = DataType{1.0, 2.0, 3.0};
	   auto B = DataType{4.0, 5.0, 6.0};
	   DataType result;
	   auto d_result =
	       cuda::memory::make_unique_region(device, 1 * sizeof(DataType));
 
	   auto sp_result = d_result.as_span<DataType>();
	   stream.enqueue.kernel_launch(vectorAddValue, launch_config, A, B,
	                                sp_result.data());
	   stream.enqueue.copy(h_result, d_result);
	   stream.synchronize();
	   print_result(A, B, result);

However, passing them as span, or pointer works perfectly fine, even though the code should also be asserted if such argument is forbidden.

I must admit I perfectly understand Your point of view: cuda-api-wrappers framework should make the cuda code safer, which means we should avoid any Undefined Behaviors, however assertion can be bypassed or disables the usage of commonly used library.

My idea here is to allow usage of custom traits for libraries which user can enable when
User code must ensure that this workflow does not affect program correctness.

This could be implemented like:

template<typename T>
struct is_gpu_compatible : std::is_trivially_copy_constructible<T> {};


static_assert(
    detail_::all_true< 
        ::is_gpu_compatible< detail_::kernel_parameter_decay_t<KernelParameters> >::value...
    >::value,
    "All kernel parameter types must be GPU-compatible."
);

And then I could use this trait in my code:

namespace my_library_traits {

template<>
struct is_gpu_compatible<Eigen::Matrix<float, 3, 1, 0, 3, 1>> : std::true_type {};

} // namespace my_library_traits

[eyalroz]

This sounds like a reasonable suggestion. I'd bikeshed the trait name, though, perhaps something like cuda::detail_::valid_kernel_argument<T>. Would you like to create a PR for this?

If not, I should get around to it within a couple of weeks I think.

Note to self: Need to add a clarification about this in the doxygen comments for the kernel launching functions and methods, so that users know how to bypass the default check, if necessary.

[ralwing]

Yep, I'll create a PR for this, we need one more clarification though. Although the name of the traits is completely fine, I think that putting it into details namespace would make the user code

namespace cuda::details
{ template <> struct is_gpu_compatible<Eigen::Vector> : ::std::true_type {}; }  // namespace cuda

using some internal tricks. So my suggestion is to put it either in a traits namespace :

namespace cuda::traits
{
 template <> struct valid_kernel_argument<T> : ::std::true_type {};
}  // namespace cuda

or simply in cuda:: namespace.

Furthermore, the static_assert desciption should be changed. What Do You think about:

	"All kernel parameter types must be function argument compatible (decayed) type. "
	"See https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#global-function-argument-processing");

[eyalroz]

Here is my rationale (although I'm not putting my foot down, just thinking out loud for now):

I have so far not intended to expose type traits as an exported constructs of the library, and indeed, the are currently no traits outside cuda::detail_. In fact, the are very few facts even _inside cuda::detail_ (look at the type_traits.hpp header). Moreover, the declaration of an exception ot the type trait I usually check for is not exactly "user" code. I think of it more as part of the code of Eigen; or some utility code that the user is provided with and should not need to worry about. How would a lay user know whether Eigen::Vector is safe to copy ot the GPU or not? That's not at all obvious.

---

PS - cuda::detail_, not cuda::details

[ralwing]

Thanks for clarification.

the are currently no traits outside cuda::detail_

That's right, however, this is a customization trait, not an internal trait. That's why I insist on putting it into a more "general" namespace.

I think of it more as part of the code of Eigen; or some utility code that the user is provided with and should not need to worry about.

Sounds great! I'm unsure if it is possible to implement it currently. If even NVIDIA hasn't provided such a trait, it's likely beyond current C++ capabilities.
Just off the top of my head is allowing user to customize the copy operations, e.g. like fmt::formatter class allows custom types to be formatted.

How would a lay user know whether Eigen::Vector is safe to copy ot the GPU or not? That's not at all obvious.

Agreed, exposing that trait risks hard-to-find bugs. However, users who understand such trait are aware of the consequences. For example, I used unit tests to verify CUDA-copyable classes.

[eyalroz]

(I'm ok with it, I think - was expecting your pull request.)

[eyalroz]

@ralwing : ping.

[ralwing]

Sorry, but i'm currently a bit overwhelmed. I think I will be able to make it next week.

[ralwing]

Implemented in 4ddc7fa

[eyalroz]

Not closed until this lands on master... I marked this as "resolved on development"