[SYCL][clang] Fix more free-function kernel integration header cases #20877

steffenlarsen · 2025-12-11T13:51:38Z

This commit fixes a number of known issues when the integration header generates prototypes of the free-function kernels. These changes focus on the additional removal of aliasing and proper handling of templated template arguments.

This commit also adds disabled test cases for a known issue with unresolved nested templated type aliases. These are cases for future work.

This commit fixes a number of known issues when the integration header generates prototypes of the free-function kernels. These changes focus on the additional removal of aliasing and proper handling of templated template arguments. This commit also adds disabled test cases for a known issue with unresolved nested templated type aliases. These are cases for future work. Signed-off-by: Larsen, Steffen <[email protected]> Co-authored-by: Sachkov, Alexey <[email protected]>

Fznamznon

Just questions, will continue with meaningful review on Monday.
@elizabethandrews , @premanandrao given the size of the pr you're weclome to review as well.

clang/include/clang/AST/TypeBase.h

clang/lib/AST/Type.cpp

clang/include/clang/AST/TypeBase.h

Fznamznon · 2025-12-12T15:50:28Z

clang/lib/Sema/SemaSYCL.cpp


      const TemplateSpecializationType *TSTAsNonAlias =
-          TST->getAsNonAliasTemplateSpecializationType();
+          TST->getAsTemplateSpecializationTypeWithoutAliases(Context);


So I was kind of expecting that the visitor soluiton would help us to avoid unwrapping and modifying types manually like getAsTemplateSpecializationTypeWithoutAliases does. Is that not the case? Could you please elaborate why do we need such a complex solution?

This patch was the marriage between a patch @AlexeySachkov made and one that I prepared. The solution is still needed to avoid aliases, but it has now been integrated into the visitor, which I indeed think is the better solution. Good thinking!

clang/lib/Sema/SemaSYCL.cpp

Signed-off-by: Larsen, Steffen <[email protected]>

…t_header_cases

Signed-off-by: Larsen, Steffen <[email protected]>

This commit fixes the desugaring of independent aliased argument types in dependent free function kernels. Signed-off-by: Larsen, Steffen <[email protected]>

Fznamznon · 2025-12-15T17:57:35Z

clang/lib/Sema/SemaSYCL.cpp

+                                        ASTContext &Context)
+      : O(O), Policy(Policy), Context(Context) {}
+
+  void Visit(const TemplateSpecializationType *T,


Could you please give T and CT meaningful names or add a comment explaining what is what?

Fznamznon · 2025-12-15T17:59:55Z

clang/lib/Sema/SemaSYCL.cpp

+
+    const TemplateDecl *TD = CT->getTemplateName().getAsTemplateDecl();
+    if (!TD->getIdentifier())
+      TD = T->getTemplateName().getAsTemplateDecl();


Is T guaranteed to have an identifier?

I can't think of a case where it doesn't, but that doesn't mean it's impossible. I can introduce an unreachable or similar to error out instead of generating an invalid identifier if that is better?

Fznamznon · 2025-12-15T18:05:56Z

clang/lib/Sema/SemaSYCL.cpp

+
+  void VisitDeclarationTemplateArgument(const TemplateArgument &,
+                                        ArrayRef<TemplateArgument>) {
+    llvm_unreachable("Free function kernels cannot have non-type template "


Does it make sense to diagnose this case prior integration header emission? Like in SyclKernelFieldChecker.

Fznamznon · 2025-12-15T18:07:21Z

clang/lib/Sema/SemaSYCL.cpp

+
+  void VisitTemplateExpansionTemplateArgument(const TemplateArgument &Arg,
+                                              ArrayRef<TemplateArgument>) {
+    // Likely does not work similar to the one above


Is that a FIXME?

Fznamznon · 2025-12-15T19:34:03Z

clang/lib/Sema/SemaSYCL.cpp

+      // default argument.
+      // We pass specialized arguments in case there are references to them
+      // from other types.
+      // FIXME: passing SpecArgs here is incorrect. It refers to template


That FIXME does seem concerning.

I agree, though the current version of this still addresses a chunk of the current issues we are seeing with the prototype generation. I can try to add some more disabled cases to the test so we know what to fix.

Fznamznon · 2025-12-15T19:36:10Z

clang/lib/Sema/SemaSYCL.cpp

+  }
+
+  void VisitExpressionTemplateArgument(const TemplateArgument &Arg,
+                                       ArrayRef<TemplateArgument>) {


It would be great to explain somewhere why all the methods additionally accept an ArrayRef that sometimes ends up unused.

Fznamznon · 2025-12-15T19:36:45Z

clang/lib/Sema/SemaSYCL.cpp

+    assert(E && "Failed to get an Expr for an Expression template arg?");
+
+    if (Arg.isInstantiationDependent() ||
+        E->getType().getTypePtr()->isScopedEnumeralType()) {


can we

Suggested change

E->getType().getTypePtr()->isScopedEnumeralType()) {

E->getType()->isScopedEnumeralType()) {

steffenlarsen requested a review from a team as a code owner December 11, 2025 13:51

steffenlarsen temporarily deployed to WindowsCILock December 11, 2025 13:51 — with GitHub Actions Inactive

steffenlarsen temporarily deployed to WindowsCILock December 11, 2025 14:24 — with GitHub Actions Inactive

steffenlarsen had a problem deploying to WindowsCILock December 11, 2025 14:24 — with GitHub Actions Failure

steffenlarsen temporarily deployed to WindowsCILock December 11, 2025 14:24 — with GitHub Actions Inactive

steffenlarsen temporarily deployed to WindowsCILock December 11, 2025 16:12 — with GitHub Actions Inactive

Fznamznon reviewed Dec 12, 2025

View reviewed changes

steffenlarsen added 3 commits December 14, 2025 22:51

Remove getAsTemplateSpecializationTypeWithoutAliases

bea8af0

Signed-off-by: Larsen, Steffen <[email protected]>

Merge remote-tracking branch 'intel/sycl' into steffen/fix_more_ff_in…

14b14d5

…t_header_cases

Integrate desugaring

3c7f972

Signed-off-by: Larsen, Steffen <[email protected]>

steffenlarsen temporarily deployed to WindowsCILock December 15, 2025 07:37 — with GitHub Actions Inactive

steffenlarsen temporarily deployed to WindowsCILock December 15, 2025 08:06 — with GitHub Actions Inactive

Desugar aliases in independent free function arguments

abbb457

This commit fixes the desugaring of independent aliased argument types in dependent free function kernels. Signed-off-by: Larsen, Steffen <[email protected]>

steffenlarsen temporarily deployed to WindowsCILock December 15, 2025 11:45 — with GitHub Actions Inactive

steffenlarsen temporarily deployed to WindowsCILock December 15, 2025 12:20 — with GitHub Actions Inactive

steffenlarsen had a problem deploying to WindowsCILock December 15, 2025 12:20 — with GitHub Actions Failure

Fznamznon reviewed Dec 15, 2025

View reviewed changes

	E->getType().getTypePtr()->isScopedEnumeralType()) {
	E->getType()->isScopedEnumeralType()) {

[SYCL][clang] Fix more free-function kernel integration header cases #20877

Are you sure you want to change the base?

[SYCL][clang] Fix more free-function kernel integration header cases #20877

Conversation

steffenlarsen commented Dec 11, 2025

Uh oh!

Fznamznon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants