-
Notifications
You must be signed in to change notification settings - Fork 740
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SYCL][clang] Emit default template arguments in integration header #16005
[SYCL][clang] Emit default template arguments in integration header #16005
Conversation
For free function kernels support clang forward declares the kernel itself as well as its parameter types. In case a free function kernel has a parameter that is templated and has a default template argument, all template arguments including arguments that match default arguments must be printed in kernel's forward declarations, for example ``` template <typename T, typename = int> struct Arg { T val; }; // For the kernel SYCL_EXT_ONEAPI_FUNCTION_PROPERTY( (ext::oneapi::experimental::nd_range_kernel<1>)) void foo(Arg<int> arg) { arg.val = 42; } // Integration header must contain void foo(Arg<int, int> arg); ``` Unfortunately, even though integration header emission already has extensive support for forward declarations priting, some modifications to clang's type printing are still required. infrastructure, since neither of existing PrintingPolicy flags help to reach the correct result. Using `SuppressDefaultTemplateArgs = true` doesn't help without printing canonical types, printing canonical types for the case like ``` template <typename T> SYCL_EXT_ONEAPI_FUNCTION_PROPERTY( (ext::oneapi::experimental::nd_range_kernel<1>)) void foo(Arg<T> arg) { arg.val = 42; } // Printing canonical types is causing the following integration header template <typename T> void foo(Arg<type-parameter-0-0, int> arg); ``` Using `SkipCanonicalizationOfTemplateTypeParms` field of printing policy doesn't help here since at the one point where it is checked we take canonical type of `Arg`, not its parameters and it will contain template argument types in canonical type after that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests for work group memory arguments used with free function kernels are now passing with these changes so for me this looks good.
Policy.FullyQualifiedName = true; | ||
Policy.EnforceScopeForElaboratedTypes = true; | ||
|
||
// SuppressDefaultTemplateArguments is a downstream addition that suppresses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think an example in the comments would be helpful here. Probably the same one in PR description. Just reading these is very confusing without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added, thanks!
@elizabethandrews , are you ok with the change? |
@intel/llvm-gatekeepers , the PR is ready for submit. Please merge. |
For free function kernels support clang forward declares the kernel itself as well as its parameter types. In case a free function kernel has a parameter that is templated and has a default template argument, all template arguments including arguments that match default arguments must be printed in kernel's forward declarations, for example
Unfortunately, even though integration header emission already has extensive support for forward declarations priting, some modifications to clang's type printing are still required, since neither of existing PrintingPolicy flags help to reach the correct result.
Using
SuppressDefaultTemplateArgs = true
doesn't help without printing canonical types, printing canonical types for the case likeUsing
SkipCanonicalizationOfTemplateTypeParms
field of printing policy doesn't help here since at the one point where it is checked we take canonical type ofArg
, not its parameters and it will contain template argument types in canonical type after that.