From 09512d857820fcb525490e862f7f9d1f547a2bf5 Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Thu, 28 May 2020 13:22:17 -0700 Subject: [PATCH 1/2] Fix KernelNameInfo generated in integration header when kernel name type is a template specialization class with empty template pack argument. This commit fixes a bug introduced by commit e7020a17439b322bfb where an extra comma was printed before empty pack arguments Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 21 ++++++++++++++------- clang/test/CodeGenSYCL/int_header1.cpp | 9 +++++++++ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 997c24bb9636c..1ad76d9afa76f 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1864,16 +1864,20 @@ static void printArguments(ASTContext &Ctx, raw_ostream &ArgOS, const PrintingPolicy &P); static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, - TemplateArgument Arg, const PrintingPolicy &P) { + TemplateArgument Arg, const PrintingPolicy &P, + bool FirstArg) { switch (Arg.getKind()) { case TemplateArgument::ArgKind::Pack: { + if (Arg.pack_size() && !FirstArg) + ArgOS << ", "; printArguments(Ctx, ArgOS, Arg.getPackAsArray(), P); break; } case TemplateArgument::ArgKind::Integral: { QualType T = Arg.getIntegralType(); const EnumType *ET = T->getAs(); - + if (!FirstArg) + ArgOS << ", "; if (ET) { const llvm::APSInt &Val = Arg.getAsIntegral(); ArgOS << "(" << ET->getDecl()->getQualifiedNameAsString() << ")" << Val; @@ -1889,10 +1893,14 @@ static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, TypePolicy.SuppressTagKeyword = true; QualType T = Arg.getAsType(); QualType FullyQualifiedType = TypeName::getFullyQualifiedType(T, Ctx, true); + if (!FirstArg) + ArgOS << ", "; ArgOS << FullyQualifiedType.getAsString(TypePolicy); break; } default: + if (!FirstArg) + ArgOS << ", "; Arg.print(P, ArgOS); } } @@ -1900,13 +1908,12 @@ static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, static void printArguments(ASTContext &Ctx, raw_ostream &ArgOS, ArrayRef Args, const PrintingPolicy &P) { + bool FirstArg = true; + for (unsigned I = 0; I < Args.size(); I++) { const TemplateArgument &Arg = Args[I]; - - if (I != 0) - ArgOS << ", "; - - printArgument(Ctx, ArgOS, Arg, P); + printArgument(Ctx, ArgOS, Arg, P, FirstArg); + FirstArg = false; } } diff --git a/clang/test/CodeGenSYCL/int_header1.cpp b/clang/test/CodeGenSYCL/int_header1.cpp index a2e6288a7875c..59e91a7acdb17 100644 --- a/clang/test/CodeGenSYCL/int_header1.cpp +++ b/clang/test/CodeGenSYCL/int_header1.cpp @@ -12,6 +12,7 @@ // CHECK:template <> struct KernelInfo<::nm1::KernelName4> { // CHECK:template <> struct KernelInfo<::nm1::KernelName8<::nm1::nm2::C>> { // CHECK:template <> struct KernelInfo<::TmplClassInAnonNS> { +// CHECK:template <> struct KernelInfo<::nm1::KernelName9> { // This test checks if the SYCL device compiler is able to generate correct // integration header when the kernel name class is expressed in different @@ -42,6 +43,9 @@ namespace nm1 { template <> class KernelName4 {}; template <> class KernelName4 {}; + template + class KernelName9; + } // namespace nm1 namespace { @@ -128,6 +132,10 @@ struct MyWrapper { kernel_single_task>( [=]() { acc.use(); }); + // Kernel name type is a templated specialization class with empty template pack argument + kernel_single_task>( + [=]() { acc.use(); }); + return 0; } }; @@ -151,5 +159,6 @@ int main() { KernelInfo>::getName(); KernelInfo>::getName(); KernelInfo>::getName(); + KernelInfo>::getName(); #endif //__SYCL_DEVICE_ONLY__ } From 25a1ad1b1faafce6671737729b159582a11b173b Mon Sep 17 00:00:00 2001 From: Elizabeth Andrews Date: Fri, 29 May 2020 09:26:05 -0700 Subject: [PATCH 2/2] Implemented review comments: 1. Moved Comma Handling back to printArguments() 2. Add a check for empty pack argument in printArguments() Signed-off-by: Elizabeth Andrews --- clang/lib/Sema/SemaSYCL.cpp | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 1ad76d9afa76f..f2b930932d3ce 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -1864,20 +1864,16 @@ static void printArguments(ASTContext &Ctx, raw_ostream &ArgOS, const PrintingPolicy &P); static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, - TemplateArgument Arg, const PrintingPolicy &P, - bool FirstArg) { + TemplateArgument Arg, const PrintingPolicy &P) { switch (Arg.getKind()) { case TemplateArgument::ArgKind::Pack: { - if (Arg.pack_size() && !FirstArg) - ArgOS << ", "; printArguments(Ctx, ArgOS, Arg.getPackAsArray(), P); break; } case TemplateArgument::ArgKind::Integral: { QualType T = Arg.getIntegralType(); const EnumType *ET = T->getAs(); - if (!FirstArg) - ArgOS << ", "; + if (ET) { const llvm::APSInt &Val = Arg.getAsIntegral(); ArgOS << "(" << ET->getDecl()->getQualifiedNameAsString() << ")" << Val; @@ -1893,14 +1889,10 @@ static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, TypePolicy.SuppressTagKeyword = true; QualType T = Arg.getAsType(); QualType FullyQualifiedType = TypeName::getFullyQualifiedType(T, Ctx, true); - if (!FirstArg) - ArgOS << ", "; ArgOS << FullyQualifiedType.getAsString(TypePolicy); break; } default: - if (!FirstArg) - ArgOS << ", "; Arg.print(P, ArgOS); } } @@ -1908,12 +1900,17 @@ static void printArgument(ASTContext &Ctx, raw_ostream &ArgOS, static void printArguments(ASTContext &Ctx, raw_ostream &ArgOS, ArrayRef Args, const PrintingPolicy &P) { - bool FirstArg = true; - for (unsigned I = 0; I < Args.size(); I++) { const TemplateArgument &Arg = Args[I]; - printArgument(Ctx, ArgOS, Arg, P, FirstArg); - FirstArg = false; + + // If argument is an empty pack argument, skip printing comma and argument. + if (Arg.getKind() == TemplateArgument::ArgKind::Pack && !Arg.pack_size()) + continue; + + if (I != 0) + ArgOS << ", "; + + printArgument(Ctx, ArgOS, Arg, P); } }