Skip to content

Conversation

@mfrancepillois
Copy link

Adds a missing parallel_for signature when used with sycl::kernel and user-defined properties.

…rties

Adds a missing `parallel_for` signature when used with `sycl::kernel` and properties.
@mfrancepillois
Copy link
Author

Without this added signature, this type of call goes through the generic API parallel_for(nd_range<Dims> Range, PropertiesT Properties, _KERNELFUNCPARAM(KernelFunc)) and fails on an assert in detail::CheckDeviceCopyable<KernelType>();.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maarquitos14
Copy link
Contributor

I think the signature you're adding is not exactly the same as the one the spec shows:

  template <int Dimensions>
  void parallel_for(nd_range<Dimensions> ndRange, const kernel& kernelObject);

@steffenlarsen what do you think?

Copy link
Contributor

@steffenlarsen steffenlarsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@maarquitos14 is right, this does not match any of the SYCL 2020 variants of parallel_for nor any of the ones added in sycl_ext_oneapi_kernel_properties. This was raised at one point, but due to some of the awkwardness of only really being able to apply launch-based properties, we went with only adding new overloads to sycl_ext_oneapi_enqueue_functions. Would it be possible to use that extension for these sort of launches?

@mfrancepillois
Copy link
Author

Thanks for your suggestion. The sycl_ext_oneapi_enqueue_functions extension does indeed addresses this issue of launching sycl::kernel with properties.
This PR can therefore be closed.

@mfrancepillois mfrancepillois deleted the maxime/kernel-parallel-for-properties branch October 2, 2024 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants