[PATCH] D60455: [SYCL] Implement SYCL device code outlining

[Alexey Bader via Phabricator via cfe-commits]

bader added a subscriber: erichkeane.
bader added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10079-10080
+def warn_sycl_kernel_invalid_template_param_type : Warning<
+  "template parameter of template functions with 'sycl_kernel' attribute must"
+  " be typename">, InGroup<IgnoredAttributes>;
+def warn_sycl_kernel_num_of_function_params : Warning<
----------------
aaron.ballman wrote:
> This diagnostic reads a bit like you cannot do this: `template <class N>` when I think the actual restriction is that you cannot do this: `template <int N>`. Is that correct? If so, I think this could be worded as `template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter`.
> 
> Just double-checking, but you also intend to prohibit template template parameters? e.g., you can't do `template <template <typename> typename C>`
> This diagnostic reads a bit like you cannot do this: template <class N> when I think the actual restriction is that you cannot do this: template <int N>. Is that correct?

Yes. That is correct.

>  If so, I think this could be worded as template parameter of a function template with the 'sycl_kernel' attribute must be a template type parameter.

Thanks! Applied your wording.

> Just double-checking, but you also intend to prohibit template template parameters? e.g., you can't do template <template <typename> typename C>

Currently we allow following use case: https://github.com/intel/llvm/blob/sycl/clang/test/SemaSYCL/mangle-kernel.cpp. I assume it qualifies as "template type" and not as "template template" parameter. Right?

Quoting SYCL specification $6.2 Naming of kernels (https://www.khronos.org/registry/SYCL/specs/sycl-1.2.1.pdf#page=250).

> SYCL kernels are extracted from C++ source files and stored in an implementation- defined format. In the case of
> the shared-source compilation model, the kernels have to be uniquely identified by both host and device compiler.
> This is required in order for the host runtime to be able to load the kernel by using the OpenCL host runtime
> interface.
> From this requirement the following rules apply for naming the kernels:
> • The kernel name is a C++ typename.
> • The kernel needs to have a globally-visible name. In the case of a named function object type, the name can
> be the typename of the function object, as long as it is globally-visible. In the case where it isn’t, a globally visible name has to be provided, as template parameter to the kernel invoking interface, as described in 4.8.5.
> In C++11, lambdas do not have a globally-visible name, so a globally-visible typename has to be provided
> in the kernel invoking interface, as described in 4.8.5.
> • The kernel name has to be a unique identifier in the program.
> 

We also have an extension, which lifts these restrictions/requirements when clang is used as host and device compiler. @erichkeane implemented built-in function (https://github.com/intel/llvm/pull/250) providing "unique identifier", which we use as a kernel name for lambda objects. But this is going to be a separate patch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60455/new/

https://reviews.llvm.org/D60455